Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Reduce AutoDateFormatter precision when possible#4809
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Closed
Closed
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Previously, the AutoDateFormatter would choose a format with second ormicrosecond precision, even when the ticks were significantly coarserthan that. The resulting extra precision looks weird and can clutter thedisplay (especially with the long microsecond display).This commit changes the default scale to format dictionary, which nowworks as follows: - Use microsecond precision when the ticks are less than a second apart - Use second precision when the ticks are seconds apart - Use minute precision when the ticks are minutes or hours apart - Use day-precision, month or year precision when the ticks are days or more apart (unchanged).Note that there is no point in displaying only the hour when the ticks arehours apart, since then it won't be immediately clear that a time is beingdisplayed. Adding the (technically superfluous) :00 for the minutes shouldmake it immediately obvious that a time is being displayed, which is why theminute precision should also be used when the ticks are hours apart.While updating the documentation for this change, it was also changed to usesymbolic constants instead of hardcoded numbers. This should make it more clearwhat the intention is.Closes:matplotlib#4808
Note that this pullrequest has not been tested as-is, since I don't have time right now to set up a full matplotlib build from git. I did test the changes backported into 1.4.3, which works as expected. |
| 1.0 : '%b %d %Y', | ||
| 1. / HOURS_PER_DAY : '%H:%M', | ||
| 1. / SEC_PER_DAY : '%H:%M:%S', | ||
| 1. / MUSECONDS_PER_DAY : '%H:%M:%S.%f' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This part is a doc-string, so perhaps it doesn't make sense to use the global variables here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I would say it makes the documentation more clear like this? Also, expanding MUSECONDS_PER_DAY would result in1. / (24 * 60 * 60 * 1000000), which struck me as overly complicated. But I'm happy to change back to numbers if you think that would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fair enough. I'll let others weigh in with their preference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think the globals as much more readable.
tacaswell added a commit to tacaswell/matplotlib that referenced this pull requestNov 8, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestNov 11, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestNov 16, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestNov 16, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestNov 17, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestNov 23, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestNov 25, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestNov 27, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestNov 27, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestDec 14, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestDec 14, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
Would it make sense to just combine this with#5445,@tacaswell? |
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestDec 14, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestDec 17, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestDec 31, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
mdboom pushed a commit to mdboom/matplotlib that referenced this pull requestDec 31, 2015
- Use ISO complient formats by default - aded extra level of scale (seconds) - add rcparams for all of these stringsclosesmatplotlib#4808closesmatplotlib#4809closesmatplotlib#5086
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, the AutoDateFormatter would choose a format with second or
microsecond precision, even when the ticks were significantly coarser
than that. The resulting extra precision looks weird and can clutter the
display (especially with the long microsecond display).
This commit changes the default scale to format dictionary, which now
works as follows:
apart (unchanged).
Note that there is no point in displaying only the hour when the ticks are
hours apart, since then it won't be immediately clear that a time is being
displayed. Adding the (technically superfluous) :00 for the minutes should
make it immediately obvious that a time is being displayed, which is why the
minute precision should also be used when the ticks are hours apart.
While updating the documentation for this change, it was also changed to use
symbolic constants instead of hardcoded numbers. This should make it more clear
what the intention is.
Closes:#4808