FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

IGNORE: Lockout new axis converters by jklymak · Pull Request #9736 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

IGNORE: Lockout new axis converters #9736

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

Closed
jklymak wants to merge 2 commits into from
Conversation 11 Commits 2 Checks 0 Files changed
Closed

IGNORE: Lockout new axis converters #9736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08a800c
If an axis has a non-None converter, don't allow another non-None
jklymak Nov 9, 2017
393a05b
TST: test that two different converters same axis give TypeError
jklymak Nov 9, 2017
File filter

Filter by extension

Filter by extension .py  (2) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
7 changes: 7 additions & 0 deletions lib/matplotlib/axis.py
Show comments
View file
Open in desktop
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
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -1434,6 +1434,13 @@ def update_units(self, data):
if converter is None:
return False

if (self.converter is not None) and (self.converter != converter):
raise TypeError('Attempting to plot data that is '
'registered to be converted with %s but '

story645 Nov 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should the language be something like "data with units %s which is incompatible with existing axis units %s" - kinda wondering if registered convertors is too weedy for users.

jklymak Nov 9, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Open to this - I find the word "units" very confusing in this context. Looking at the jpl stuff I can see there was an attempt to let the data have different units and then get converted by what the axis units were (i.e. if xunits='centimetres' but the x-axis units were 'meters' 1.0 would be plotted as 0.01) but that seems to have been lost along the way. Or I'm misunderstanding (more likely)

story645 Nov 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think that's sort of what the basic units example might be about too-same numbers, but we'll do the conversion internally. Maybe just stick to type? "attempting to plot data of type %s that is incompatiable with existing axis data of type %s"

jklymak Nov 9, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Oooh, except it returns fun things like matplotlib.dates.DateConverter not the data type. I could state what the new type is (i.e. type blah cannot be processed by axis unit converter matplotlib.dates.DateConverter)

dstansby Dec 27, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I like that last suggestions @jklymak

FWIW a converter that converts meters should also be able convert centimeters fine (they're both lengths!)
'is incompatible with existing axis data '
'converter %s' % (converter.__class__,
self.converter.__class__))

neednew = self.converter != converter

dstansby Dec 27, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This line can change to neednew = self.converter == None, and maybe change the variable name neednew to needupdate too since that's really what it means now.
self.converter = converter
default = self.converter.default_units(data, self)
Expand Down
11 changes: 11 additions & 0 deletions lib/matplotlib/tests/test_dates.py
Show comments
View file
Open in desktop
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
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -477,3 +477,14 @@ def test_tz_utc():
def test_num2timedelta(x, tdelta):
dt = mdates.num2timedelta(x)
assert dt == tdelta


def test_one_non_None_converter():
# test that we only allow one non-None converter at once:
base = datetime.datetime(2017, 1, 1, 0, 10, 0)
time = [base - datetime.timedelta(days=x) for x in range(0, 3)]
data = [0., 2., 4.]
fig, ax = plt.subplots()
ax.plot(time, data)
with pytest.raises(TypeError):
ax.plot(['a', 'b'], [1., 2.])
Back | FazBrowse Home | New Git URL