Skip to content

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Sep 14, 2021

See docstring change for the confusion that this resolves.
We can normalize None inputs on the caller side (and note that directly
constructing MarkerStyles is not something that most users need to do).

PR Summary

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@anntzer anntzer added this to the v3.6.0 milestone Sep 14, 2021
@anntzer anntzer force-pushed the msn2 branch 2 times, most recently from dc1e568 to 4df79be Compare September 14, 2021 19:26
Comment on lines 4120 to 4123
for y, marker in enumerate([
"none",
*sorted({*matplotlib.markers.MarkerStyle.markers} - {"none"},
key=lambda x: str(type(x))+str(x))]):
Copy link
Member

Choose a reason for hiding this comment

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

I tried but struggled to work out what's going on here with the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test simply draws all markers in a single figure, ordered by str(type(x))+str(x) (not all x have same type: markers can be str (most of them), but also int or, previously, None, hence the need to convert to str). A previous PR introduced MarkerStyle("none"), i.e. "none" was added into the MarkerStyle.markers dict; in order to not change the baseline image, I had to remove it from the list of entries. With this PR, I additionally removed None from the dict; again, in order to not change the baseline image, I had to put it back (using its synonym "none") into the list of entries (at the position where it was previously, i.e. in from of everything else).

Copy link
Member

Choose a reason for hiding this comment

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

I would write this as

    markers = sorted(matplotlib.markers.MarkerStyle.markers,
                     key=lambda x: str(type(x))+str(x))
    # Since generation of the test image, None was removed but 'none' was
    # added. By moving 'none' to the front (=former sorted place of None)
    # we can avoid regenerating the test image. This can be removed if the
    # test image has to be regenerated for other reasons.
    markers.remove('none')
    markers = ['none', *markers]

    for y, marker in enumerate(markers):

Copy link
Contributor Author

@anntzer anntzer Sep 15, 2021

Choose a reason for hiding this comment

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

sure, done

See docstring change for the confusion that this resolves.
We can normalize None inputs on the caller side (and note that directly
constructing MarkerStyles is not something that most users need to do).
@timhoffm timhoffm merged commit ded4951 into matplotlib:master Sep 16, 2021
@anntzer anntzer deleted the msn2 branch September 16, 2021 21:47
tacaswell pushed a commit that referenced this pull request Oct 20, 2021
Deprecate MarkerStyle(None).
EwoutH added a commit to quaquel/EMAworkbench that referenced this pull request Oct 23, 2022
Fix two cases of the following deprecation:

MatplotlibDeprecationWarning: MarkerStyle(None) is deprecated since 3.6; support will be removed two minor releases later.  Use MarkerStyle('') to construct an empty MarkerStyle.

See matplotlib/matplotlib#21074
EwoutH added a commit to quaquel/EMAworkbench that referenced this pull request Oct 23, 2022
Fix two cases of the following deprecation:

MatplotlibDeprecationWarning: MarkerStyle(None) is deprecated since 3.6; support will be removed two minor releases later.  Use MarkerStyle('') to construct an empty MarkerStyle.

See matplotlib/matplotlib#21074
EwoutH added a commit to quaquel/EMAworkbench that referenced this pull request Oct 23, 2022
Fix two cases of the following deprecation:

MatplotlibDeprecationWarning: MarkerStyle(None) is deprecated since 3.6; support will be removed two minor releases later.  Use MarkerStyle('') to construct an empty MarkerStyle.

See matplotlib/matplotlib#21074
EwoutH added a commit to quaquel/EMAworkbench that referenced this pull request Oct 25, 2022
Fix two cases of the following deprecation:

MatplotlibDeprecationWarning: MarkerStyle(None) is deprecated since 3.6; support will be removed two minor releases later.  Use MarkerStyle('') to construct an empty MarkerStyle.

See matplotlib/matplotlib#21074
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants