In PR#23130 it was noticed that certain example files had 'test' in their name and were getting picked up automatically bypytest accidentally. To avoid this,

matplotlib/pytest.ini

Line 10 in8e2987b

testpaths = lib

was added topytest.ini. While this is sufficient to avoid future problems, removing the word 'test' from example files and replacing it with 'demo', which is commonly used in other matplotlib examples, provides future safeguards.

~~To further this idea, uses of the phrase 'test' in the example docstrings are replaced with 'demo'.~~ (do this later on given#23219 (comment))

To avoid causing broken links in theexample documentation addredirect-from statements to the old file names asdirected in the dev docs.

edit: There are additional examples beyondexamples/scales/log_test.py withtest in the name:

examples/text_labels_and_annotations/usetex_baseline_test.py
examples/units/artist_tests.py
examples/units/evans_test.py

but it was decided these examples should be removed or turned into actual tests, and so to avoid the noise of renaming and then removing these are left alone for the scope of this PR.

PR Checklist

Tests and Styling

[N/A] Has pytest style unit tests (andpytest passes).
[N/A] IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

[N/A] New features are documented, with examples if plot related.
[N/A] New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
[N/A] API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).

Copy link

ContributorAuthor

matthewfeickert commentedJun 8, 2022

@greglucas this is ready for review.

oscargus reviewed

Jun 8, 2022

View reviewed changes

examples/units/artist_demo.py OutdatedShow resolvedHide resolved

oscargus reviewed

Jun 8, 2022

View reviewed changes

examples/units/artist_demo.py OutdatedShow resolvedHide resolved

Copy link

Member

oscargus commentedJun 8, 2022

I seem to recall that there is some sort of redirect functionality as well (basically, when accessing old versions of this it will say something like "outdated, click here for the most recent version, and it will redirect).

Here is a the relevant part of the documentaion:https://matplotlib.org/stable/devel/documenting_mpl.html?highlight=redirect#moving-documentation

matthewfeickert force-pushed themnt/rename-examples-with-test-in-name branch 2 times, most recently fromc3c3709 to873ca91Compare

June 8, 2022 06:42

Copy link

ContributorAuthor

matthewfeickert commentedJun 8, 2022

I seem to recall that there is some sort of redirect functionality as well (basically, when accessing old versions of this it will say something like "outdated, click here for the most recent version, and it will redirect).
Here is a the relevant part of the documentation:https://matplotlib.org/stable/devel/documenting_mpl.html?highlight=redirect#moving-documentation

Ah thanks for that and many thanks for the link too!

matthewfeickert requested a review fromoscargus

June 8, 2022 06:45

oscargus approved these changes

Jun 8, 2022

View reviewed changes

Copy link

Member

oscargus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks good! Thanks!

Only thing is that I cannot check (yet) that the redirects work.

oscargus added Documentation Maintenance labels

Jun 8, 2022

oscargus added this to thev3.5-doc milestone

Jun 8, 2022

Copy link

Member

oscargus commentedJun 8, 2022

I cannot really confirm that it works. Typing the old name doesn't seem to work (for the case I've tested), but maybe that is not how it is supposed to work? (Or it only works on that actual docs.)

Apart from that, this PR looks good!

timhoffm reviewed

Jun 8, 2022

View reviewed changes

Copy link

Member

timhoffm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

3 / 4 of the cases are not really examples. We should consider taking them out.

examples/text_labels_and_annotations/usetex_baseline.py Outdated

		@@ -1,8 +1,10 @@
		"""
		====================
		Usetex BaselineTest
		Usetex BaselineDemo

Copy link

Member

timhoffmJun 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This looks more like somebody has created a test image so that they can visually check that the baseline is correct. I don't think this has any instructional value. Should we remove this and instead make a real test out of it (if we don't have one)?

examples/units/artist_demo.py Outdated

		Artist tests
		============
		===========
		Artist demo

Copy link

Member

timhoffmJun 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Also really more a test than an example?

examples/units/evans_demo.py Outdated

		@@ -1,8 +1,10 @@
		"""
		==========
		Evanstest
		Evansdemo

Copy link

Member

timhoffmJun 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I don't think this is an example either. It could possibly become a tutorial "How to use custom units", but certainly needs more explanation for that. Btw. does somebody know why it's called "Evans test"?

Copy link

Member

tacaswell commentedJun 8, 2022

I suspect that these files are the remnants of the pre-nose world where the "tests" were some scripts you ran to see if they worked so they really were written to be tests. Not sure if they were accidentally or intentionally not migrated to units tests when nose was adopted?

It looks likeevans_test.py came in to being viaf2a0c7a , I strongly suggest this was an example suggest by some with some part of the name as Evan or Evans. Searching discourse from about that time getshttps://discourse.matplotlib.org/t/bar-plot-forget-units-information/7884 which will not quite the same, suggests that a James Evans was interested in units in ~2007 so it is plausible the file is named after their needs!

I think the comprise I would prefer here is to rename the files (to make pytest happy) but leave the headings as they are (to be as clear as we have currently been) about what these files are actually for.

Copy link

ContributorAuthor

matthewfeickert commentedJun 8, 2022•
edited
Loading

I think the comprise I would prefer here is to rename the files (to make pytest happy) but leave the headings as they are (to be as clear as we have currently been) about what these files are actually for.

@tacaswell SGTM. So just to be clear so I can revise this quickly:

Keep renames and redirects.
Revert the change of 'test' to 'demo' everywhere.
In a follow up PR do the migration of anything that could be a usefulpytest test as@timhoffm suggested.

Do I have that right?

Copy link

Member

tacaswell commentedJun 8, 2022

@matthewfeickert Yes.

matthewfeickert force-pushed themnt/rename-examples-with-test-in-name branch from873ca91 to71e6270Compare

June 8, 2022 22:32

tacaswell approved these changes

Jun 8, 2022

View reviewed changes

Copy link

Member

tacaswell commentedJun 8, 2022

There are unrelated warnings:

WARNING: html_theme_options['switcher']['url_template'] is no longer supported. Set version URLs in JSON directly.looking for now-outdated files... none foundpickling environment... donechecking consistency... donepreparing documents... WARNING: unsupported theme option 'logo_link' given

Copy link

ContributorAuthor

matthewfeickert commentedJun 9, 2022

There are unrelated warnings:

WARNING: html_theme_options['switcher']['url_template'] is no longer supported. Set version URLs in JSON directly.looking for now-outdated files... none foundpickling environment... donechecking consistency... donepreparing documents... WARNING: unsupported theme option 'logo_link' given

Yeah, I assume these are the same things that are getting fixed in PR#23225.

matthewfeickert force-pushed themnt/rename-examples-with-test-in-name branch from71e6270 to27bbe3aCompare

June 9, 2022 16:03

Copy link

Member

timhoffm commentedJun 9, 2022

Thinking about it, I have a slight preference of not renaming the examples that we want to remove anyway. Introducing a new name in a release is only one more potentially broken link.

AFAICS, the immidiate need for changing the name is remedied byd9fbc7d. So it's more a cleanup thing.

Sorry for going back in multiple steps. I haven't properly thought this through before.

Copy link

ContributorAuthor

matthewfeickert commentedJun 14, 2022

Thinking about it, I have a slight preference of not renaming the examples that we want to remove anyway. Introducing a new name in a release is only one more potentially broken link.

@timhoffm So in your mind the only file that should get renamed isexamples/scales/semilogx_test.py ->examples/scales/semilogx_demo.py, correct? Before I make any changes can you,@greglucas,@tacaswell, and@oscargus all confirm that you agree on this? After your have reached a consensus I'll apply any changes and rebase.

AFAICS, the immidiate need for changing the name is remedied byd9fbc7d. So it's more a cleanup thing.

Correct. c.f.this Gitter discussion.

Copy link

Member

oscargus commentedJun 14, 2022

I have no objections. What@timhoffm says seems to make sense. No point in renaming if they will be removed.

MNT: Rename example files with 'test' in name

3d864dd

In PR 23130 it was noticed that certain example files had 'test' in theirname and were getting picked up automatically by pytest accidentally. Toavoid this, `testpaths = lib` was added to pytest.ini. While this is sufficientto avoid future problems, removing the word 'test' from example file names andreplacing it with 'demo', which is commonly used in other matplotlib examples,provides future safeguards.To avoid causing broken links in the example documentationhttps://matplotlib.org/stable/gallery/index.htmladd `redirect-from` statements to the old file names as directed inhttps://matplotlib.org/stable/devel/documenting_mpl.html#moving-documentation.There are additional examples beyond 'examples/scales/log_test.py' but it was decidedthese examples should be removed or turned into actual tests, and so to avoid the noiseof renaming and then removing these are left alone for the scope of PR 23219.

matthewfeickert force-pushed themnt/rename-examples-with-test-in-name branch from27bbe3a to3d864ddCompare

June 15, 2022 08:28

matthewfeickert requested review fromtimhoffm,oscargus andtacaswell

June 15, 2022 08:28

Copy link

ContributorAuthor

matthewfeickert commentedJun 15, 2022

I've gone ahead and dropped the changes to

examples/text_labels_and_annotations/usetex_baseline_test.py
examples/units/artist_tests.py
examples/units/evans_test.py

given@timhoffm's suggestions (but I still have them on a backup branch in the event people want them again) so this is now just a single file rename (examples/scales/semilogx_test.py ->examples/scales/semilogx_demo.py) with a redirect.