Skip to content

Fixing missing coordinate unit in aperture_photometry return for single ... #213

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

Jump to bottom
Merged
bsipocz merged 1 commit into from
Dec 20, 2014
Merged

Fixing missing coordinate unit in aperture_photometry return for single ... #213

bsipocz merged 1 commit into from
Dec 20, 2014

Conversation

@bsipocz
Copy link
Member

@bsipocz bsipocz commented Dec 17, 2014

...positions

Sadly hub is not working out of the box, thus I couldn't attached this directly to #212.
Part of the problem causing this bug was that the inner container of SkyCoord is not as consistent as Quantity's for single and multiple elements. I'm not sure whether this is a feature or just accidental, if the latter I'll open an issue for it in core.

In [325]: coord_columns_m
Out[324]: 
(<Quantity [ 99., 99.] pix>,
 <Quantity [ 49., 49.] pix>,
 <SkyCoord (ICRS): (ra, dec) in deg
     [(0.0, 0.0), (0.0, 0.0)]>)  

In [325]: coord_columns_s
Out[325]: 
(<Quantity [ 99.] pix>,
 <Quantity [ 49.] pix>,
 <SkyCoord (ICRS): ra=0.0 deg, dec=0.0 deg>)

fixes #212

@cdeil
Copy link
Member

cdeil commented Dec 17, 2014

@bsipocz - Thanks!

Do you have a self-contained code snippet for your question if there's an issue to be filed for SkyCoord?
Can you put that as a separate Astropy issue or ask on the mailing list?

@cdeil
Copy link
Member

cdeil commented Dec 17, 2014

Is this ready to merge?
Can you attach a regression test here or do you have that in some other branch?

@cdeil cdeil added this to the 0.1 milestone Dec 17, 2014
@bsipocz
Copy link
Member Author

bsipocz commented Dec 17, 2014

@cdeil - yes, I'll make a self contained version of it for the core, just wondered first whether you think this is a problem or not. I think it could be more like Quantity for single elements, like the example below, but I may well miss a trivial point with it:

<SkyCoord (ICRS): (ra, dec) in deg
     [(0.0, 0.0)]>

@cdeil
Copy link
Member

cdeil commented Dec 17, 2014

@bsipocz - I don't understand what the issue was / is here.

If you want me to check this out before you ask somewhere else, can you post some code I can execute and mention what you think is not nice about how SkyCoord behaves?

@cdeil cdeil mentioned this pull request Dec 17, 2014
Merged
@cdeil
Copy link
Member

cdeil commented Dec 17, 2014

Restarted travis-ci ... this was an unrelated issue I just now fixed in master.

@larrybradley
Copy link
Member

larrybradley commented Dec 20, 2014

@bsipocz Are you still working on this?

@bsipocz
Copy link
Member Author

bsipocz commented Dec 20, 2014

@larrybradley - No, this should be fine as is.

@larrybradley
Copy link
Member

larrybradley commented Dec 20, 2014

OK, great. Can you please rebase this? It looks like travis-ci is failing because of the missing jinja dependency, which is now fixed in master.

@bsipocz bsipocz force-pushed the aperture_fix_unit_single_position branch from f7c6a1d to 7146c4d Compare December 20, 2014 02:30
bsipocz added a commit that referenced this pull request Dec 20, 2014
@bsipocz
Merge pull request #213 from bsipocz/aperture_fix_unit_single_position
c9caf3e 
Fixing missing coordinate unit in aperture_photometry return table
@bsipocz bsipocz merged commit c9caf3e into astropy:master Dec 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@bsipocz bsipocz

Labels

aperture bug

Projects

None yet

Milestone

0.1

Development

Successfully merging this pull request may close these issues.

Unit of position doesn't return from aperture_photometry for singe positions

3 participants

@bsipocz @cdeil @larrybradley