10000 Missing return in _num_to_string() by FedeMiorelli · Pull Request #8594 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Missing return in _num_to_string() #8594

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
May 9, 2017
Merged

Missing return in _num_to_string() #8594

merged 1 commit into from
May 9, 2017

Conversation

FedeMiorelli
Copy link
Contributor

Added a missing return statement that caused "None" strings to be displayed with LogFormatter.

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/whats_new.rst if major new feature
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@FedeMiorelli
Missing return in _num_to_string()
17f4b41 
Added a missing return statement that caused "None" strings to be displayed with LogFormatter.
@WeatherGod
Copy link
Member

I am a bit concerned that nobody has noticed this before? This method appears to be used in the LogFormatter's __call__ method!

@story645
Copy link
Member
story645 commented May 8, 2017 •
edited
Loading

I've noticed it before with other formatters but thought it was expected behavior. Wonder if that's what's going on?

@FedeMiorelli can you add a test that this now yields the expected labeling? You can add it to the LogFormatter tests.

@tacaswell tacaswell added this to the 2.0.2 (critical bug fixes from 2.0.1) milestone May 9, 2017
@tacaswell
Copy link
Member

That does appear to be very broken...

Looks like it was broken in 8bdabcd

The tests we have of this either a) test fmt.pprint_val directly or b) only check that tick labels are non-empty strings or not.

Going to merge, backport this, and create a new issue for adding the tests.

@tacaswell tacaswell merged commit 74fa27b into matplotlib:master May 9, 2017
tacaswell added a commit that referenced this pull request May 9, 2017
@tacaswell
Merge pull request #8594 from FedeMiorelli/patch-1
e273f7e 
FIX: Missing return in LogFormatter._num_to_string()
@tacaswell
Copy link
Member

backported to v2.0.x as e273f7e

@tacaswell tacaswell mentioned this pull request May 9, 2017
Closed
@efiring
Copy link
Member
efiring commented May 9, 2017

Wow! I really blew that one. Thanks for finding and fixing it.

@FedeMiorelli FedeMiorelli deleted the patch-1 branch May 9, 2017 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
55A1 No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.0.2 (critical bug fixes from 2.0.1)
Development

Successfully merging this pull request may close these issues.
5 participants
@FedeMiorelli @WeatherGod @story645 @tacaswell @efiring
0