GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Mnt/multi imageset by tacaswell · Pull Request #25734 · matplotlib/matplotlib

Conversation

@tacaswell

@tacaswell tacaswell commented Apr 20, 2023

Copy link
Copy Markdown
Member

PR Summary

This is not ready for review at all yet, but opening so that I can refer to it in the readme of https://github.com/tacaswell/mpl-imageset-demo. This code is .... sub optimal and needs to be refactored (the compare method should not sometimes write out baseline images and sometimes do comparisons!), but it

The rough scheme is as such:

  1. replace the baseline directory with a text file (currently called image_list.txt where each line is a : separated tuple of (relative path to file, revision number, timestamp)
  2. using git blame we can extract the last commit where any given line was changed
  3. via ENV (and eventually better interfaces) you can re-direct where the image comparison machinery looks for images
  4. in one mode running the test suite will generate the baseline images and write out a json file noting the rev number (from the file), the sha the image was last changed in, and the current version of Matplotlib
  5. in the normal mode the test suite will read the json file and verify that the image in the baseline images is consistent with what we get from image_list.txt

An example of the image list file looks like

test_a/A.pdf:1:1681408722.5270584
test_a/A.png:0:1681408661.3898573
test_a/A.svg:0:1681408661.3898566
test2/B.pdf:0:1681408661.389891
test2/B.png:0:1681408661.3898895

The logic on this format is :

  • use : as a separator because it forbidden in windows paths the leading paths will be relative (so no drive letters)
  • information must be in one line so git blame is easy to reason about
  • version rev number is for humans to read and reason about
  • the timestamp is to ensure that two people versioning reving the same image in different PRs will cause a merge conflict. There needs to be something semi-unique here. An alternative might be to add a short note justifying why, your initials, asking people to hit random keys, using uuid4, or ... would also work.

The json dumped in the generated baseline directory looks like

Details
{
  "test_basic/image.png": {
    "mpl_version": "3.8.0.dev913+g8ce98ade16.d20230420",
    "rev": 0,
    "sha": "8691c2d2bf868a23c80f6ac85ba184a917e49f03"
  },
  "test_basic/line.pdf": {
    "mpl_version": "3.8.0.dev913+g8ce98ade16.d20230420",
    "rev": 0,
    "sha": "8691c2d2bf868a23c80f6ac85ba184a917e49f03"
  },
  "test_basic/line.png": {
    "mpl_version": "3.8.0.dev913+g8ce98ade16.d20230420",
    "rev": 0,
    "sha": "8691c2d2bf868a23c80f6ac85ba184a917e49f03"
  },
  "test_basic/line.svg": {
    "mpl_version": "3.8.0.dev913+g8ce98ade16.d20230420",
    "rev": 0,
    "sha": "8691c2d2bf868a23c80f6ac85ba184a917e49f03"
  },
  "test_other/hist.png": {
    "mpl_version": "3.8.0.dev913+g8ce98ade16.d20230420",
    "rev": 1,
    "sha": "0000000000000000000000000000000000000000"
  },
  "test_other/scatter.pdf": {
    "mpl_version": "3.8.0.dev913+g8ce98ade16.d20230420",
    "rev": 0,
    "sha": "8691c2d2bf868a23c80f6ac85ba184a917e49f03"
  },
  "test_other/scatter.png": {
    "mpl_version": "3.8.0.dev913+g8ce98ade16.d20230420",
    "rev": 0,
    "sha": "8691c2d2bf868a23c80f6ac85ba184a917e49f03"
  },
  "test_other/scatter.svg": {
    "mpl_version": "3.8.0.dev913+g8ce98ade16.d20230420",
    "rev": 0,
    "sha": "8691c2d2bf868a23c80f6ac85ba184a917e49f03"
  }
}

The sha is for the computer, the rev is for the human, and the mpl version is for debugging.

open questions

  • how do deal with non-git checkouts. As this is mostly going to be around released versions so I think that narrows the problem space quite a bit.
  • tooling around updating these
  • how to post diffs when images are changed
  • how to generate cached version of the baseline on merge to main
  • how to generate and distribute "blessed" versions on tag

PR Checklist

Documentation and Tests

  • Has pytest style unit tests (and pytest passes)
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • New plotting related features are documented with examples.

Release Notes

  • New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
  • API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
  • Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anntzer reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@tacaswell tacaswell added this to the v3.8.0 milestone Apr 20, 2023
@tacaswell

tacaswell commented Apr 21, 2023

Copy link
Copy Markdown
Member Author

See tacaswell/mpl-imageset-demo#1 for what a PR would look like

see https://github.com/tacaswell/mpl-imageset-demo#operation for a skeletal user guide but it covers:

  • how to run tests
  • how to regenerate the baseline images
  • how to validate that the baseline set is consistent with the current checkout
  • tell the system we have changed a test image
  • tell the system about a new test image

Presumably manage_baseline_images.py will end up in our tools directory and the the envs will be folded into a pytest plugin / some other way to thread that configuration through.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added the status: needs rebase label May 16, 2023
@tacaswell
tacaswell force-pushed the mnt/multi_imageset branch from c047f0b to 8dab81f Compare May 18, 2023 16:20
@github-actions github-actions Bot removed the status: needs rebase label May 18, 2023
@tacaswell
tacaswell force-pushed the mnt/multi_imageset branch 2 times, most recently from 26911a3 to 4a0d7ad Compare May 18, 2023 21:20
@tacaswell

tacaswell commented May 19, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Of course there are complications 🤣

  • 2 of the eps tests actually compare against a pdf output (and there is some special casing in the baseline lookup code for that (has been there from ~2011). Given that we will no longer be storing the files, we can probably drop that long term
  • think I have fixed the windows vs posix path issues

Tasks:

  • pull apart the "compare" and "generate" functionality
  • integrate with pytest test discovery to when in generate mode do not run non-image tests (save time!)
  • try to find better internal names for everything in the compare / generate methods
  • write docs on how to bootstrap your own test images
  • write docs of how to add image
  • write docs of how to remove and image
  • write docs on how to update an image
  • write helper to zip up and systematically name a set of images
  • sort out how to leverage caching (on all of the CI services)
  • write GHA to generate above zip/tar balls of images (or do git)
  • ..and push those someplace (?)
  • make a wheel with the images (?)
  • write docs on how to use pre-generated images
  • look at sqlite instead of json of md cache
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tacaswell

tacaswell commented May 19, 2023

Copy link
Copy Markdown
Member Author

The image_lists.txt for Matplotlib are now checked in so people can take a look at what those look like at full scale.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ksunden

ksunden commented May 19, 2023

Copy link
Copy Markdown
Member

Should the metadata.json files be included in the repo? or are those intended to be local only?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tacaswell

tacaswell commented May 19, 2023

Copy link
Copy Markdown
Member Author

Should the metadata.json files be included in the repo? or are those intended to be local only?

They are intended to go with the generated sets of images, however given that we are not at the point where we can actually pull the images out of the repo, but I still want the tests to run they need to be checked in for now. The final version of this PR will squash them out of existence.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tacaswell

tacaswell commented May 19, 2023

Copy link
Copy Markdown
Member Author

I think this is at a point where (modulo the metadata.json files) where it is starting to be reviewable.

The internal names are...not great, but I think I have pulled it apart enough that it is not complete spaghetti code.

MPLTESTIMAGEPATH='/tmp/test_images2' MPLGENERATEBASELINE=1 pytest

should work and generate you a full tree of test images on this branch!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ksunden ksunden mentioned this pull request Jun 15, 2023
Start using Cirrus CI #24597
Closed
6 tasks
@tacaswell
tacaswell force-pushed the mnt/multi_imageset branch from 8da8195 to 5afeca3 Compare June 15, 2023 18:58
@github-actions github-actions Bot added the status: needs rebase label Jun 20, 2023
@tacaswell
tacaswell force-pushed the mnt/multi_imageset branch from 5afeca3 to 6125208 Compare June 21, 2023 01:10
tacaswell added 14 commits June 20, 2023 21:11
@tacaswell
MNT: py312 deprecates pickling objects in itertools
e908a41
@tacaswell
FIX: also account for itertools.count used in _AxesStack
7c1e630
@tacaswell
CI: skip tk tests on GHA as well
94e0988
@tacaswell
Revert "Revert " Merge pull request matplotlib#4019 from myshen/annot…
0fe8d37
…_neg_coords""

This partially reverts commit 7b71257.

Too many tests were removed, restore the extra tests.
@tacaswell
TST: not clear how this ever worked
9c21fdc
@tacaswell
TST: update test images for text baseline changes
803b4c8
@tacaswell
WIP: add a ENV to look for the baseline images out-of-tree
54a04c5
@tacaswell
TST: add ENV to cause the test suite to generate baseline images
94518d2
@tacaswell
DOC: start to sketch documentation
4b8f248
@tacaswell
WIP: barely working proof-of-concept
4d13089
@tacaswell
MNT: handle creating the metadata json file if needed
b1b8677
@tacaswell
MNT: make keys Path objects
69bf1e1
@tacaswell
MNT: refactor directory generation functions
d8dc57c
@tacaswell
TST: initial checking of list of test images
eadd0ac
SidharthBansal and others added 13 commits June 20, 2023 21:12
@SidharthBansal @tacaswell
ENH: add pytest collector to only select image tests to run
d36c6f8
@SidharthBansal @tacaswell
Shorten the flag name
c5b6287
@SidharthBansal @tacaswell
ENH: Added Pytest Plugin to run image generation
bb7d8a0
@tacaswell
MNT: remove errors on baseline images missing
cffa655
Flagging that we should generate images via the plugin means we
do not have access at import time to if we intend to generate the images
so remove this check.
@tacaswell
MNT: change the default libpath to match Matplotlib
5516ddc
@tacaswell
WIP: improve cli tool
9680166
@tacaswell
MNT: switch to using ts*100 as int for image list timestamp
406bc96
The factor of 100 reduces the window of collisions to 10ms which is an
acceptable risk.
@tacaswell
MNT: add new test image
2d1bb5f
@tacaswell
WIP: regenerate json
30f4519
Force this out of existence eventually
@tacaswell
MNT: remove images from image_list
cccab00
@tacaswell
WIP
db78223
@tacaswell
MNT: use - instead of _ in CLI flags
74f69b8
@tacaswell
ENH: add pytest flag + restore default values via ENV
cab2a30
@tacaswell
tacaswell force-pushed the mnt/multi_imageset branch from 6125208 to cab2a30 Compare June 21, 2023 01:13
@github-actions github-actions Bot removed the status: needs rebase label Jun 21, 2023
@tacaswell

tacaswell commented Jun 21, 2023

Copy link
Copy Markdown
Member Author

This now includes a pytest plugin (😱) so that

pytest --image-baseline=/tmp/test_images --generate-images   # generate the baseline images
pytest --image-baseline=/tmp/test_images -n 20               # test against them

works. The $MPLTESTIMAGEPATH and $MPLGENERATEBASELINE also still work (but the flags "win").

All of these names need some feedback.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tacaswell added 2 commits June 20, 2023 22:22
@tacaswell
PRF: add naive caching
39480ab
@tacaswell
PERF: skip existing images when generating
639ab9b
@tacaswell tacaswell modified the milestones: v3.8.0, v3.9.0 Jun 21, 2023
@tacaswell

tacaswell commented Jun 21, 2023

Copy link
Copy Markdown
Member Author

This seems to be working (successfully generated test images with a new freetype and it passed against them)!

I'm now sure that this is going to work, but lots of details left.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added the status: needs rebase label Aug 2, 2023
@tacaswell tacaswell modified the milestones: v3.9.0, v3.10.0 Mar 13, 2024
@tacaswell

tacaswell commented Mar 13, 2024

Copy link
Copy Markdown
Member Author

not going to get this done in the next few weeks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tacaswell tacaswell modified the milestones: v3.10.0, v3.11.0 Sep 18, 2024
@tacaswell tacaswell mentioned this pull request Mar 28, 2025
Proposal for the baseline images problem #16447
Open
@tacaswell tacaswell modified the milestones: v3.11.0, future releases Sep 11, 2025

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

status: needs rebase

Projects

None yet

Milestone

future releases

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@tacaswell @ksunden @SidharthBansal
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.