8000 DOC: Better error when float on datetime axis by jklymak · Pull Request #10084 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

DOC: Better error when float on datetime axis #10084

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 28, 2017

Conversation

jklymak
Copy link
Member
@jklymak jklymak commented Dec 24, 2017

PR Summary

#9211 points out that the error message after

import matplotlib.pyplot as plt
from datetime import datetime

fig, ax = plt.subplots()
ax.plot(datetime(2017, 1, 1), 1)
ax.plot(1, 1)
plt.show()

yields the mysterious error:

ValueError: ordinal must be >= 1

This PR catches earlier and returns:

ValueError: view limit minimum -36815.450000000004 < 1. This often happens if you pass a float to an axis that has datetime units

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


if dmin < 1:
raise ValueError('datalim minimum {} < 1. This often '
'happens if you pass a float to an '
Copy link
Contributor

Choose a reason for hiding this comment

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

indent

raise ValueError('cannot convert {} < 1 to a date. This '
'often happens if floats are passed to '
'a Matplotlib axis that expects datetime objects. '
''.format(ix))
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need the '', starting the line with a dot binds to the string on the previous line

if ix < 1:
raise ValueError('cannot convert {} < 1 to a date. This '
'often happens if floats are passed to '
'a Matplotlib axis that expects datetime objects. '
Copy link
Contributor

Choose a reason for hiding this comment

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

to an axis (no need to mpl, which you also did not include below)

Copy link
Contributor
@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

minor formatting/wording can be improved but nothing blocking

@jklymak jklymak force-pushed the doc-better-data-error branch from f02880f to 50969c4 Compare December 24, 2017 19:49

if dmin < 1:
raise ValueError('datalim minimum {} < 1. This often '
'happens if you pass a float to an '
Copy link
Member

Choose a reason for hiding this comment

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

Can "float" be change to something less technical like "number" or "normal number"?

Copy link
Member
@story645 story645 Dec 25, 2017

Choose a reason for hiding this comment

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

value that is not a datetime?

@dstansby
Copy link
Member

(apart from my suggested change I am +10000 on this, since I remember in my early Matplotlib days spending ages meaning what on earth the current error message meant)

@jklymak
Copy link
Member Author
jklymak commented Dec 26, 2017

Yep I’ll fix it this pm. No problem making it even clearer.

@jklymak jklymak force-pushed the doc-better-data-error branch from 50969c4 to 88a19c2 Compare December 26, 2017 22:55
@jklymak
Copy link
Member Author
jklymak commented Dec 26, 2017

Done - I couldn't get a test to work, however. 🐑 If I do:

g, ax = plt.subplots()
ax.plot(datetime(2017, 1, 1), 1)
ax.plot(1, 1)
fig.draw(fig.canvas.get_renderer())

I get the error. Trying to do this in dates_test.py I get:

  def test_bad_datetime():
        with pytest.raises(ValueError):
            fig, ax = plt.subplots()
            ax.plot(datetime.datetime(2017, 1, 1), 1)
            ax.plot(1, 1)
>           fig.draw(fig.canvas.get_renderer())
E           Failed: DID NOT RAISE <class 'ValueError'>

@@ -282,6 +283,11 @@ def _from_ordinalf(x, tz=None):
tz = _get_rc_timezone()

ix = int(x)
if ix < 1:
raise ValueError('cannot convert {} < 1 to a date. This '
Copy link
Member

Choose a reason for hiding this comment

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

I think should be:
cannot convert {} to a date

{} < 1 evaluate to a bool 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. Changed to

                             'view limit minimum {} is less than 1 and '
                             'is an invalid Matplotlib date value. This '
                             'often happens if you pass a non-datetime '
                             'value to an axis that has datetime units'
                             .format(vmin))

@jklymak jklymak force-pushed the doc-better-data-error branch from 88a19c2 to 38b137b Compare December 26, 2017 23:49
@jklymak jklymak force-pushed the doc-better-data-error branch from 38b137b to a4759e8 Compare December 27, 2017 05:12
@dstansby dstansby added this to the v2.2 milestone Dec 28, 2017
@dstansby dstansby merged commit dfc3a79 into matplotlib:master Dec 28, 2017
pauldmccarthy added a commit to pauldmccarthy/fsleyes that referenced this pull request Jan 3, 2018
@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 12, 2018
@jklymak jklymak deleted the doc-better-data-error branch April 8, 2018 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0