Skip to content
This repository has been archived by the owner on Jun 16, 2018. It is now read-only.

Fix subtle unit bug for angular axes #171

Merged
merged 5 commits into from
Jul 20, 2015
Merged

Conversation

astrofrog
Copy link
Owner

Fix use case where user requests longitude/latitude types for coordinates but underlying WCS is not truly celestial, but has angular units for axes.

This could be optimized by simply setting the scalefactor to apply when set_coord_type is called. Also, this needs a regression test.

@Cadair - I think this fixes your bug?

I'm also heavily jet lagged, so this will have to wait until I'm not before merging :)

@astrofrog
Copy link
Owner Author

@Cadair - can you confirm whether this fixes your issue?

@Cadair
Copy link
Contributor

Cadair commented Jul 12, 2015

@astrofrog This seems to fix the axes labels but not the mouse over, also the overlay grid doesn't seem to render properly.

@astrofrog
Copy link
Owner Author

@Cadair - does this fix it now? I'm not happy with this solution, but at least I have a regression test so I can try and find more elegant ways to do this.

@Cadair
Copy link
Contributor

Cadair commented Jul 20, 2015

👍

This seems to fix it.

So how much beer am I going to have to promise to buy you to persuade you to release a bug fix version once this is merged so we can use it for SunPy 0.6?! ;)

@astrofrog astrofrog force-pushed the fix-noncelestial-angular-wcs branch from 06e9220 to a7d7fca Compare July 20, 2015 11:51
@astrofrog
Copy link
Owner Author

I take payments in cider not beer - is that ok? 😉

I'll release later today. I'm not 100% happy with the current solution, but it works and doesn't break anything, so will merge this then open an issue to remind to work on a better solution.

Note to self: the issue is fundamentally that the transform classes don't keep track of units, and the reason for this is because the methods on this function need to be able to return Nx3 arrays where each column will be in a different unit, but we have to easy way to deal with that with Astropy units.

@Cadair
Copy link
Contributor

Cadair commented Jul 20, 2015

The cider - beer equivalency.... I will add it to your account for babysitting my hoodie 😉

astrofrog added a commit that referenced this pull request Jul 20, 2015
@astrofrog astrofrog merged commit 95ec1e5 into master Jul 20, 2015
@astrofrog
Copy link
Owner Author

@Cadair - done! https://pypi.python.org/pypi/wcsaxes/0.6 🍺 🍺 🍺

@Cadair
Copy link
Contributor

Cadair commented Jul 20, 2015

@astrofrog where did 0.5 go?!

Also, you are awesome!!

@astrofrog
Copy link
Owner Author

0.5 -> oops

@Cadair
Copy link
Contributor

Cadair commented Jul 20, 2015

lol

astrofrog added a commit that referenced this pull request Nov 28, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants