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

Add example creating GWCS from quantities and times. #695

Merged
merged 4 commits into from
May 8, 2024

Conversation

samaloney
Copy link
Contributor

PR Description

Add an example showing how to create a GWCS and NDCube from quantities and times.

@samaloney samaloney force-pushed the doc-quantity-gwcs branch from c4fe84f to cc9b9a7 Compare May 2, 2024 13:43
@nabobalis nabobalis added this to the 2.3.0 milestone May 2, 2024
@samaloney samaloney changed the title Add example createig GWCS from quanties and times. Add example creating GWCS from quantities and times. May 2, 2024
@samaloney samaloney force-pushed the doc-quantity-gwcs branch from cc9b9a7 to af9053d Compare May 2, 2024 15:19
# Create new `~ndcube.extra_coords.extra_coords.ExtraCoords` instance and add the previously created table coordinates
# and extract the gWCS.

extra_coords = ExtraCoords()
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 interesting, I hadn't even thought of doing this.

That being said, I think I would prefer we document direct use of the Table coordinate classes like in the recent example of the AIA wavelength stack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So like this wcs = CompoundLowLevelWCS(energy_coord.wcs, time_coord.wcs)
the only thing here implicit order to get the right mapping between pixel and physical it a bit more explicit with the ExtraCoords?

# Create new `~ndcube.wcs.wrappers.compound_wcs.CompoundLowLevelWCS` instance using the previously created table
# coordinates WCSs (note the ordering).

wcs = CompoundLowLevelWCS(time_coord.wcs, energy_coord.wcs, )
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
wcs = CompoundLowLevelWCS(time_coord.wcs, energy_coord.wcs, )
wcs = (time_coord & energy_coord).wcs

Would be the idiomatic way to do this iirc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh nice I might leave the more verbose way was a comment or comment or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the cube at the end is the same but the object that created is a different, I've added both for the moment but only use the latter.

wcs = CompoundLowLevelWCS(time_coord.wcs, energy_coord.wcs)
wcs
<ndcube.wcs.wrappers.compound_wcs.CompoundLowLevelWCS object at 0x156ecce50>
CompoundLowLevelWCS Transformation
This transformation has 2 pixel and 2 world dimensions
Array shape (Numpy order): None
Pixel Dim  Axis Name  Data size  Bounds
        0  None            None  None
        1  None            None  None
World Dim  Axis Name  Physical Type  Units
        0  time       time           s
        1  energy     em.energy      keV
Correlation between pixel and world axes:
           Pixel Dim
World Dim    0    1
        0  yes   no
        1   no  yes

wcs2 = (time_coord & energy_coord).wcs
<WCS(output_frame=CompositeFrame, input_frame=PixelFrame, forward_transform=Model: CompoundModel
Inputs: ('x0', 'x1')
Outputs: ('y0', 'y1')
Model set size: 1
Expression: [0] & [1]
Components: 
    [0]: <Tabular1D(points=(<Quantity [0., 1., 2., 3., 4., 5., 6., 7., 8.] pix>,), lookup_table=[0. 1. 2. 3. 4. 5. 6. 7. 8.] s)>
    [1]: <Tabular1D(points=(<Quantity [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.] pix>,), lookup_table=[0. 1. 2. 3. 4. 5. 6. 7. 8. 9.] keV)>
Parameters:)>

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't recommend using CompoundLowLevelWCS unless you really have to. It creates a wrapper layer between two gWCS objects where it really should only be one gWCS object. Also there seems to be some serious performance overhead in CompoundLowLevelWCS, which I have never investigated. It's main use case is to join two different APE 14 WCSes together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I've removed the CompoundLowLevelWCS approach.

@samaloney samaloney force-pushed the doc-quantity-gwcs branch 3 times, most recently from 650a8c7 to a43674d Compare May 8, 2024 13:40
@samaloney samaloney force-pushed the doc-quantity-gwcs branch from a43674d to eb98a8e Compare May 8, 2024 13:53
@nabobalis nabobalis merged commit 0cc3229 into sunpy:main May 8, 2024
19 checks passed
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