Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing CLI and INI options for a consistent API #181

Merged
merged 14 commits into from
Oct 19, 2023

Conversation

astrofrog
Copy link
Collaborator

@astrofrog astrofrog commented Sep 8, 2022

It would be nice to be able to customize the default tolerance and style to use globally as both have defaults set by 'historical reasons' that aren't necessarily sensible.

For style, one of the issues is that actually style could be more than just a string - e.g. a dict and so on. Is there a way to set global options such as these in e.g. conftest.py? (if so I can document that too).

Remaining TODOs (if we go ahead with this):

  • Add some documentation about the options
  • Add tests

@astrofrog astrofrog changed the title rAdded options to set default tolerance and style Added options to set default tolerance and style Sep 8, 2022
@Cadair
Copy link
Contributor

Cadair commented Sep 8, 2022

Is there a way to set global options such as these in e.g. conftest.py?

You can set these options in the pytest config (normally setup.cfg) but I don't know how that interacts with richer python objects? maybe we would need to parse a string in to a python object?

@ConorMacBride
Copy link
Member

Is there a way to set global options such as these in e.g. conftest.py?

You can set these options in the pytest config (normally setup.cfg) but I don't know how that interacts with richer python objects? maybe we would need to parse a string in to a python object?

The pytest global options (probably) only supports returning one of these types. So I think it would be best to just recommend adding custom rc params to a new style file within the test directory, whose path (or URL) can just be referred to in the global pytest option as a string.

Copy link
Member

@ConorMacBride ConorMacBride left a comment

Choose a reason for hiding this comment

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

These changes should get the tests passing. I think we should do a new major version soon with the default style changed from "classic" to "default", as I think any changes to the Matplotlib default style are outside the scope of this package and should be made apparent by default when testing on different Matplotlib version.

pytest_mpl/plugin.py Outdated Show resolved Hide resolved
pytest_mpl/plugin.py Outdated Show resolved Hide resolved
@ConorMacBride
Copy link
Member

ConorMacBride commented May 20, 2023

Hi @astrofrog, there are some other options (noted in #198) which could be added to make the config more consistent. Do you agree we should add these options?

  • Add mpl-baseline-path ini option (relative to where pytest was run).
  • Add --mpl-use-full-test-name CLI option.
  • Add mpl-hash-library ini option (relative to where pytest was run).
  • Add --mpl-default-tolerance CLI and mpl-default-tolerance ini options.
  • Add --mpl-default-style CLI and mpl-default-style ini options.
  • Add --mpl-default-backend CLI and mpl-default-backend ini options.
  • Add mpl-generate-summary ini option.

If so, would you like me to open a separate PR, or shall I just add them to this PR?

@astrofrog
Copy link
Collaborator Author

Sounds good - feel free to just add them to this PR if you like!

@ConorMacBride ConorMacBride changed the title Added options to set default tolerance and style Add missing CLI and INI options for a consistent API May 29, 2023
@ConorMacBride ConorMacBride force-pushed the default-tolerance-style branch 2 times, most recently from 1096aae to fd3ca4e Compare October 7, 2023 21:27
Adds tests for the new config options across ini, CLI and kwarg where relevant. Asserts that they work and have the expected order of precedence. Still need to add dedicated tests for all the other options.
Adds tests for the new config options across ini, CLI and kwarg where relevant. Asserts that they work and have the expected order of precedence. Still need to add dedicated tests for all the other options.
@ConorMacBride ConorMacBride marked this pull request as ready for review October 7, 2023 22:01
Copy link
Collaborator Author

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

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

Thanks for all the work on this!

@astrofrog astrofrog merged commit 0d11e7b into matplotlib:main Oct 19, 2023
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting the default tolerance globally
3 participants