Skip to content
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

Do not call shiftdata for scattter plots #202

Closed
wants to merge 8 commits into from
Closed

Do not call shiftdata for scattter plots #202

wants to merge 8 commits into from

Conversation

guziy
Copy link
Contributor

@guziy guziy commented Aug 21, 2015

I've added a test for the scatter function and modified the decorator to eliminate the call to shiftdata for scatter in 1-d case, since I am not really sure why it is needed....

Addresses the issue #197.

Cheers

@guziy
Copy link
Contributor Author

guziy commented Aug 27, 2015

The tests are added to show that it works with 1 and 2 points. (#191)

@pelson
Copy link
Member

pelson commented Sep 3, 2015

@WeatherGod - did you have a case where this is not a good idea?

@WeatherGod
Copy link
Member

I don't think so. It was a few years ago that I reviewed this original
feature. I can't remember why we applied it to scatter plots. The only
thing I can think of is one is doing an image and scatter. One would want
to be able to have the domains for them match up, right?

On Thu, Sep 3, 2015 at 5:07 AM, Phil Elson [email protected] wrote:

@WeatherGod https://github.com/WeatherGod - did you have a case where
this is not a good idea?


Reply to this email directly or view it on GitHub
#202 (comment).

@guziy
Copy link
Contributor Author

guziy commented Sep 4, 2015

@WeatherGod I'll try to generate a test case for this, to see if it really is a problem.

Cheers

@guziy
Copy link
Contributor Author

guziy commented Sep 6, 2015

Apparently, shiftdata is needed for shifting when longitudes are further than +/- 180 from the lon_0:

https://github.com/guziy/PyNotebooks/blob/master/test_scatter_and_shiftdata.ipynb

I was supposing that it is done in the m(lon, lat) call....

One way to patch it would be a special shiftdata for scatter, which does not care about relations, but only changes coordinates to be within lon_0 - 180 and lon_0 + 180 range... Or change the call method, however, I am not sure how it would impact other plotting methods.

Cheers

@guziy
Copy link
Contributor Author

guziy commented Sep 6, 2015

The plot with shiftdata does not seem right when you look at the latitudes of the rightmost part...

@guziy
Copy link
Contributor Author

guziy commented Sep 21, 2015

I was thinking about this a bit more, I think there should be a better way to solve this, not hacking only scatter and then maybe others... I'll try to understand shiftdata and fix it if possible...

Cheers

@guziy guziy closed this Sep 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants