Skip to content

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

Merged
bsipocz merged 1 commit into
astropy:masterfrom
bsipocz:aperture_fix_unit_single_position
Dec 20, 2014
Merged

Fixing missing coordinate unit in aperture_photometry return for single ...#213
bsipocz merged 1 commit into
astropy:masterfrom
bsipocz:aperture_fix_unit_single_position

Conversation

@bsipocz

@bsipocz bsipocz commented Dec 17, 2014

Copy link
Copy Markdown
Member

...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

cdeil commented Dec 17, 2014

Copy link
Copy Markdown
Member

@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

cdeil commented Dec 17, 2014

Copy link
Copy Markdown
Member

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

bsipocz commented Dec 17, 2014

Copy link
Copy Markdown
Member Author

@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

cdeil commented Dec 17, 2014

Copy link
Copy Markdown
Member

@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

cdeil commented Dec 17, 2014

Copy link
Copy Markdown
Member

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

@larrybradley

larrybradley commented Dec 20, 2014

Copy link
Copy Markdown
Member

@bsipocz Are you still working on this?

@bsipocz

bsipocz commented Dec 20, 2014

Copy link
Copy Markdown
Member Author

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

@larrybradley

larrybradley commented Dec 20, 2014

Copy link
Copy Markdown
Member

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