8000 Boring style fixes. by anntzer · Pull Request #11680 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Boring style fixes. #11680

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
Jul 17, 2018
Merged

Boring style fixes. #11680

merged 1 commit into from
Jul 17, 2018

Conversation

anntzer
Copy link
Contributor
@anntzer anntzer commented Jul 17, 2018

PR Summary

PR Checklist

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

jklymak
jklymak approved these changes Jul 17, 2018
View reviewed changes
Copy link
Member
@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

Take or leave the ordering issue...

lib/matplotlib/ticker.py Outdated
if decade_min <= 0 <= decade_max:
ticklocs.append(0.5)
if decade_max >= 1:
if 1 <= decade_max:
Copy link
Member

Choose a reason for hiding this comment

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

huh?

lib/matplotlib/ticker.py Outdated
if decade_min <= 0 <= decade_max:
ticklocs.extend([0.2, 0.3, 0.4, 0.6, 0.7, 0.8])
if decade_max >= 2:
if 2 <= decade_max:
Copy link
Member

Choose a reason for hiding this comment

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

This is backwards to the way most people think. I guess you want to write it this way so the numbers are in order, and I wouldn't hold up the PR on this, but...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, that was from a while ago, undoing both of these...

tacaswell
tacaswell approved these changes Jul 17, 2018
View reviewed changes
Copy link
Member
@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

modulo CI passing.

@tacaswell tacaswell modified the milestones: v3.0, v3.1 Jul 17, 2018
@jklymak
Copy link
Member
jklymak commented Jul 17, 2018

Huh, appveyor has actually passed this PR, but it shows up as pending on GH. Merging as clearly passing.

@jklymak jklymak merged commit ac97e14 into matplotlib:master Jul 17, 2018
@anntzer anntzer deleted the style branch July 17, 2018 20:37
@tacaswell tacaswell modified the milestones: v3.1, v3.0 Jul 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell approved these changes

@jklymak jklymak jklymak approved these changes

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

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