Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Allow invalid limits when panning#9363

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

Merged

Conversation

anntzer
Copy link
Contributor

PR Summary

Fixes#9361.@2sn please give it a try?
The "culprit" was#8474... this PR adds an escape hatch.

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

@anntzeranntzer added the Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labelOct 11, 2017
@anntzeranntzer added this to the2.1.1 (next bug fix release) milestoneOct 11, 2017
@2sn
Copy link

2sn commentedOct 11, 2017

I would be happy to try this, alone I cannot find any instructions how to get this change into the maplotlib git repo I just downloaded for 30 min. I am sure it is easy to do, but I cannot guess how to. There no git has provided, etc. If it is even in the git main repo ... sorry, I just have never used this.

@2sn
Copy link

2sn commentedOct 11, 2017

I managed to get you changes on top of current master. No improvement. I firmly believe all "upgrades" on this should just be undone until this can get under control. Here the error message I get from my code which is too long to share:

In [3]: Traceback (most recent call last):  File "/home/alex/matplotlib/lib/matplotlib/cbook/__init__.py", line 389, in process    proxy(*args, **kwargs)  File "/home/alex/matplotlib/lib/matplotlib/cbook/__init__.py", line 227, in __call__    return mtd(*args, **kwargs)  File "/home/alex/matplotlib/lib/matplotlib/backend_bases.py", line 3116, in release_zoom    self._zoom_mode, twinx, twiny)  File "/home/alex/matplotlib/lib/matplotlib/axes/_base.py", line 3650, in _set_view_from_bbox    self.set_xlim((x0, x1))  File "/home/alex/matplotlib/lib/matplotlib/axes/_base.py", line 2932, in set_xlim    self.viewLim, "intervalx", emit, auto, kw)  File "/home/alex/matplotlib/lib/matplotlib/axes/_base.py", line 2842, in _set_lim    "Axis limits must be (or convert to) finite reals")ValueError: Axis limits must be (or convert to) finite reals

@2sn
Copy link

2sn commentedOct 11, 2017

The other problem is that I have a user-created axis that does not plot at all in mpl 2.1, and there is not even an error message. I believe this is due to some silencing of error messages that was also done in 2.1 for the sake of, akamatplotlib.cbook.CallbackRegistry.process() suppresses exceptions by default as listed on the API changes. It remains unmentioned how or where I can influence this as a user.

@2sn
Copy link

2sn commentedOct 11, 2017

OK, I could find the reason for one of the issues, the behaviour of of ax.transData has changed from 2.0.2 to 2.1. If y-axis is log and a y-value of 0 is provided, it returns np.nan also for the x-value. This is a bug and should be fixed. Probably a separate bug report should be filed?

@2sn
Copy link

2sn commentedOct 11, 2017
edited
Loading

After identifying#9369 your patch seems to fix my problem. Thanks! When this is included, I assume you may close#9361.

return converted_limit
def _set_lim(self, axis, low, high, low_name, high_name,
target_obj, target_attr, emit, auto, kw):
"""Helper factoring the functionality of `get_{x,y,z}lim`."""
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 a private method, but I still think it would be good to write quick one line descriptions of all the args.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

done

"""Helper factoring the functionality of `get_{x,y,z}lim`."""
# Perhaps we can use axis.{get,set}_view_interval()... but zaxis seems
# to behave differently.
_called_from_pan = kw.pop("_called_from_pan", False)
Copy link
Member

Choose a reason for hiding this comment

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

It would be clearer to just addcalled_from_pan as it's own kwarg with a default given it's the only kwarg, then the next two lines aren't needed to check for extra kwargs.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Actually I decided to just do the check from the drag_pan function rather than trying to pass it as a private kwarg throught the stack...

dstansby reacted with thumbs up emoji
@anntzeranntzerforce-pushed theignore-invalid-limits-in-pan branch 3 times, most recently from9a7a247 to05e383aCompareOctober 12, 2017 20:54
@anntzer
Copy link
ContributorAuthor

The latest version actually works better than the pre-broken version: it doesn't even bother passing invalid values toset_xlim anymore.

@tacaswell
Copy link
Member

I like the direction this is going 👍

@anntzeranntzerforce-pushed theignore-invalid-limits-in-pan branch from05e383a tob848e8aCompareOctober 12, 2017 23:32
@anntzer
Copy link
ContributorAuthor

Turns out the set_?lim unification is a mess, because custom axes typically don't define get_${axisname}lim so you need to chase them down to see who's using what name.
Stripped that part out for now, will probably be the object of a separate PR later.
Now we still get warnings about invalid values but at least it should behave similarly to 2.0.2.

@dopplershift
Copy link
Contributor

Was the change to ban calling set_?lim with NaNs intentional? That's creating new challenges for us with real-world, real-time messy data. I've got a script that worked in 2.0 that now fails with 2.1 when we calculate limits from data that's all missing.

@anntzer
Copy link
ContributorAuthor

The issue was discussed (with a quick vote...) on#7460. I don't think the previous behavior was particularly helpful, but feel free to revisit it...

@dopplershift
Copy link
Contributor

Thanks for pointing me to that...looks like I need to fix things on our end.

@tacaswell
Copy link
Member

Independent of if we should treatnp.nan asNone in setting the limits, this avoids the whole problem when panning.

@dstansbydstansby merged commitefbaded intomatplotlib:masterOct 23, 2017
@anntzeranntzer deleted the ignore-invalid-limits-in-pan branchOctober 23, 2017 09:31
tacaswell added a commit that referenced this pull requestOct 23, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell approved these changes

@dstansbydstansbydstansby approved these changes

Assignees
No one assigned
Labels
Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
v2.1.1
Development

Successfully merging this pull request may close these issues.

2.1 change - Axis Limit Error
5 participants
@anntzer@2sn@tacaswell@dopplershift@dstansby

[8]ページ先頭

©2009-2025 Movatter.jp