Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Removal of baseline images from matplotlib_baseline_images package#17793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

SidharthBansal
Copy link
Contributor

@SidharthBansalSidharthBansal commentedJun 29, 2020•
edited
Loading

PR Summary

  • Generated a matplotlib.tests wheel that can be uploaded to PyPI as a separate PyPI package ("distribution", in distutils parlance), to make it possible to install test data from PyPI.
    This is useful e.g. for mplcairo, whose test suite relies on matplotlib's one.
    Also, useful for Baseline image problem optimisation.
  • Created thetest_data folder needed for additional test images apart from the baseline images
  • Created the logic for the generation of the baseline images on fresh install of matplotlib
  • In case, the baseline images exists exist and have changed then the logic for modification of the baseline images is created.
  • Creation of a plugin to generate images for bothmatplotlib andmpl_toolkits folders
  • Added related documentation.

Fixes#16447

attn:@anntzer

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@SidharthBansalSidharthBansalforce-pushed theremoval_of_baseline_images_from_baseline_package branch 2 times, most recently from25eb8c5 to678b778CompareJune 30, 2020 15:03
@SidharthBansalSidharthBansalforce-pushed theremoval_of_baseline_images_from_baseline_package branch 3 times, most recently fromc5b1486 toa000051CompareJuly 1, 2020 14:26
stdout=subprocess.DEVNULL),
universal_newlines=True)
# Clone mpl repo to tmpvenv and run pytests from new mpl repo created
subprocess.run(["git", "clone", str(mplRepoPath), "tmp/testenv"],
Copy link
Member

Choose a reason for hiding this comment

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

What version of git do we getworktree? I suspect that would be both faster and small footprint.

SidharthBansal reacted with thumbs up emoji
stderr=subprocess.STDOUT,
stdout=subprocess.DEVNULL,
universal_newlines=True)
subprocess.run(["python", "-mpip", "install", "-e", mplRepoPath],
Copy link
Member

Choose a reason for hiding this comment

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

should this be the new venv python?

stderr=subprocess.STDOUT,
stdout=subprocess.DEVNULL,
universal_newlines=True)
subprocess.run(["python", "-mpytest"],
Copy link
Member

Choose a reason for hiding this comment

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

Should also install pytest and all of the extra dependencies (pandas, ...) to make sure we generate all of the images or should we only match what the user has installed in their development environment?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I think installing all extra dependencies is a better option as testing aims to check integrity amongst the components too, which can't be achieved if only some of the installs are done.
First time all the images will be generated, later on we can move on specific missing installs and modifications of the baseline images.

Copy link
ContributorAuthor

@SidharthBansalSidharthBansalJul 2, 2020•
edited
Loading

Choose a reason for hiding this comment

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

What do you think@anntzer ?

@SidharthBansalSidharthBansalforce-pushed theremoval_of_baseline_images_from_baseline_package branch 5 times, most recently from7a6aedd toc885729CompareJuly 6, 2020 10:26
@SidharthBansal
Copy link
ContributorAuthor

Series of commands executed in the terminal:

python3 -mpytest lib/matplotlib/tests -m "baseline_image_generation_test"mkdir lib/matplotlib/tests/baseline_imagescp -r result_images/test_agg lib/matplotlib/tests/baseline_images/python3 -mpytest lib/matplotlib/tests -m "baseline_image_generation_test"

The first pytest command generates the baseline images with thebaseline images not found error. Then, in the second pytest command, tests passes.
Similar approach for mpl_toolkits_tests need to be worked upon.

@SidharthBansalSidharthBansalforce-pushed theremoval_of_baseline_images_from_baseline_package branch 5 times, most recently from631f056 tocd7dda9CompareJuly 15, 2020 03:59
@SidharthBansalSidharthBansalforce-pushed theremoval_of_baseline_images_from_baseline_package branch 2 times, most recently from15d0e90 to6982bb1CompareSeptember 7, 2020 11:45

Baseline image generation
-------------------------
The idea is to eventually remove all of the images but we have options about where devs get the baselines from.
Copy link
Member

Choose a reason for hiding this comment

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

This needs a paragraph explaining the current state of things to the reader. To us (who have been thinking about this all summer), this makes perfect sense, but to a new contributor who does not even know wehave baseline images checked into the repo this is going to be consufing.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Done

@SidharthBansalSidharthBansalforce-pushed theremoval_of_baseline_images_from_baseline_package branch 3 times, most recently fromf29ecf8 toc010250CompareSeptember 8, 2020 15:14
@SidharthBansalSidharthBansalforce-pushed theremoval_of_baseline_images_from_baseline_package branch 3 times, most recently from430fc90 to14f0c9aCompareSeptember 9, 2020 08:40
@SidharthBansalSidharthBansalforce-pushed theremoval_of_baseline_images_from_baseline_package branch from14f0c9a to57bfa89CompareSeptember 9, 2020 11:27
@tacaswelltacaswell modified the milestones:v3.4.0,v3.5.0Jan 27, 2021
@jklymakjklymak marked this pull request as draftMay 8, 2021 20:44
@jklymak
Copy link
Member

@anntzer this seems like a good start, but is there any reason to keep it open?

@anntzer
Copy link
Contributor

Let's close this for now; we can always revisit...

@anntzeranntzer closed thisMay 8, 2021
@QuLogicQuLogic modified the milestones:v3.5.0,unassignedMay 10, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell requested changes

@anntzeranntzeranntzer left review comments

@jklymakjklymakjklymak left review comments

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Proposal for the baseline images problem
5 participants
@SidharthBansal@jklymak@tacaswell@anntzer@QuLogic
[8]ページ先頭
©2009-2025 Movatter.jp