Skip to content

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

Merged
astrofrog merged 14 commits into
matplotlib:mainfrom
astrofrog:default-tolerance-style
Oct 19, 2023
Merged

Add missing CLI and INI options for a consistent API#181
astrofrog merged 14 commits into
matplotlib:mainfrom
astrofrog:default-tolerance-style

Conversation

@astrofrog

@astrofrogastrofrog commented Sep 8, 2022

Copy link
Copy Markdown
Collaborator

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
astrofrogforce-pushed the default-tolerance-style branch from b83ea27 to fe4f4e3CompareSeptember 8, 2022 09:07
@astrofrogastrofrog changed the title rAdded options to set default tolerance and styleAdded options to set default tolerance and styleSep 8, 2022
@Cadair

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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.

@ConorMacBrideConorMacBride left a comment

Copy link
Copy Markdown
Member

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.

Comment threadpytest_mpl/plugin.py Outdated
Comment threadpytest_mpl/plugin.py Outdated
@ConorMacBride

ConorMacBride commented May 20, 2023

Copy link
Copy Markdown
Member

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
Copy Markdown
CollaboratorAuthor

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

@ConorMacBride
ConorMacBrideforce-pushed the default-tolerance-style branch from f43443e to 718308aCompareMay 20, 2023 20:25
@ConorMacBrideConorMacBride changed the title Added options to set default tolerance and styleAdd missing CLI and INI options for a consistent APIMay 29, 2023
@ConorMacBride
ConorMacBrideforce-pushed the default-tolerance-style branch 2 times, most recently from 1096aae to fd3ca4eCompareOctober 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
ConorMacBrideforce-pushed the default-tolerance-style branch from fd3ca4e to a36ac38CompareOctober 7, 2023 21:49
@ConorMacBride
ConorMacBride marked this pull request as ready for review October 7, 2023 22:01

@astrofrogastrofrog left a comment

Copy link
Copy Markdown
CollaboratorAuthor

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:mainOct 19, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Setting the default tolerance globally

3 participants

@astrofrog@Cadair@ConorMacBride