8000 Simpler and stricter process_plot_format. by anntzer · Pull Request #12968 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Simpler and stricter process_plot_format. #12968

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 22, 2018
Merged

Simpler and stricter process_plot_format. #12968

merged 1 commit into from
Dec 22, 2018

Conversation

anntzer
Copy link
Contributor
@anntzer anntzer commented Dec 10, 2018

See changelog entry.

The default values were removed from the docstring as they are not true,
and depend not trivially on rcParams and effectively on the property
cycle.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 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

@anntzer anntzer added this to the v3.1 milestone Dec 10, 2018
@anntzer anntzer force-pushed the process_plot_format branch from 48a5bd4 to a7d1514 Compare December 10, 2018 08:22
timhoffm
timhoffm reviewed Dec 10, 2018
View reviewed changes
lib/matplotlib/axes/_base.py Outdated
elif c == 'C' and i < len(chars) - 1:
color_cycle_number = int(chars[i + 1])
elif c == 'C' and i < len(fmt) - 1:
color_cycle_number = int(fmt[i + 1])
color = mcolors.to_rgba("C{}".format(color_cycle_number))
i += 1
Copy link
Member
@timhoffm timhoffm Dec 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I would inline the increment completely into every if block. Having a global +1 and sometimes an additional +1 is more confusing than +1 or +2 in every block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed and changed

@anntzer
Simpler and stricter process_plot_format.
753e53d 
See changelog entry.

The default values were removed from the docstring as they are not true,
and depend not trivially on rcParams and effectively on the property
cycle.
@anntzer anntzer force-pushed the process_plot_format branch from a7d1514 to 753e53d Compare December 10, 2018 19:30
@jklymak jklymak merged commit cdc3ab1 into matplotlib:master Dec 22, 2018
@anntzer anntzer deleted the process_plot_format branch December 22, 2018 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jklymak jklymak jklymak approved these changes

@timhoffm timhoffm timhoffm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@anntzer @jklymak @timhoffm
0