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

Addition of asdf schema for our coordinate frames and GenericMap #2366

Merged
merged 34 commits into from Jan 3, 2019

Conversation

Projects
8 participants
@Cadair
Copy link
Member

commented Dec 11, 2017

This implements asdf schema for all four of our coordinate frames and also for GenericMap.

The main motivation for this is that this facilitates the use of the SunPy coordinate frames in the gWCS / asdf stack which is highly likely to be used by DKIST. The reason these schema are being PRed here is that it makes the most logical sense for the schema to live in the same repo as the classes they describe.

This is still entirely optional and if asdf is not installed then this has zero impact on SunPy.

@pep8speaks

This comment has been minimized.

Copy link

commented Dec 11, 2017

Hello @Cadair! Thanks for updating the PR.

Comment last updated on September 10, 2018 at 10:51 Hours UTC

@Cadair Cadair changed the title Initial implementation of asdf schema and tag for map [WIP] Initial implementation of asdf schema and tag for map Dec 11, 2017

@Cadair Cadair added io map [WIP] labels Dec 11, 2017

@Cadair Cadair force-pushed the Cadair:asdf_tags branch 2 times, most recently from 76acbc7 to 3b52f5e Dec 11, 2017

@dpshelio
Copy link
Member

left a comment

Looking good!!

Show resolved Hide resolved sunpy/io/special/asdf/tags/map/generic_map.py
node['shift'] = u.Quantity(smap.shifted_value)

# TODO: Save some or all of plot_settings
# node['plot_settings'] = smap.plot_settings

This comment has been minimized.

Copy link
@dpshelio

dpshelio Dec 12, 2017

Member

will that be saving a colourmap, or just the name of that colour map?

This comment has been minimized.

Copy link
@Cadair

Cadair Dec 12, 2017

Author Member

Saving the actual colourmap object (and norm) would be much harder, but possible. This is the main reason it's a todo.

@Cadair Cadair force-pushed the Cadair:asdf_tags branch from 3b52f5e to b653b8e Dec 13, 2017

@Cadair Cadair added this to High Priority Features in SunPy 0.9 Dec 13, 2017

@Cadair Cadair force-pushed the Cadair:asdf_tags branch from b653b8e to 9b5ee5b Jan 8, 2018

__all__ = ['SunpyExtension']


SUNPY_SCHEMA_URI_BASE = 'http://sunpy.org/schemas/'

This comment has been minimized.

Copy link
@wafels

wafels Jan 8, 2018

Member

Link does not work.

This comment has been minimized.

Copy link
@Cadair

Cadair Jan 8, 2018

Author Member

it's not supposed to. Well it dosen't have to.

@wafels

This comment has been minimized.

Copy link
Member

commented Jan 8, 2018

What is ASDF and why is it useful for SunPy?

@wafels

This comment has been minimized.

@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Jan 8, 2018

Some other made up standard no one will follow.

@Cadair

This comment has been minimized.

Copy link
Member Author

commented Jan 8, 2018

@wafels Your link is correct.

asdf is a file format designed to be a subsitute for FITS where FITS isn't up to the task. For a reasonably quick introduction to why it exists, watch this lightning talk from pyastro15.

As for why it's useful for SunPy, there are a few reasons, at a high level first, DKIST is (probably) going to be using it to store the metadata for whole observations (individual images will be in FITS) and as such will need to be able to save things like SunPy coordinate frames to asdf. The most logical place for those schemas and tags to live is in SunPy with the implementations of the frames.

With regards to this PR, this is a (working) proof of concept to show how integration with asdf would work for SunPy. This adds the ability to save a map to asdf, and unlike when you save a map to FITS it also saves out Map.shift and I would eventually like to make it save Map.plot_settings as well.

@dpshelio
Copy link
Member

left a comment

I'm happy to have it!! People can then start to use it and become familiar with it.

@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2018

@wafels

This comment has been minimized.

Copy link
Member

commented Jan 10, 2018

Not really in favor of supporting file formats that have no data available in that format BUT if this really is going to be the choice for upcoming instruments that have large amounts of complex data then I suppose it is OK. I would not spend a great deal of time on this at the moment when the use case at present is very thin.

@Cadair

This comment has been minimized.

Copy link
Member Author

commented Jan 10, 2018

Or we use hdf5

hdf5 does not provide even close to the same feature set, and has other technical disadvantages. That's not to say HDF5 isn't useful, it's awesome, but there is a big difference between it and both FITS and asdf.

Not really in favor of supporting file formats that have no data available

As I mentioned on the call, I think this provides a useful mechanism for pickle like serialisation of SunPy objects to disk, in a way that is much more safe and portable than pickle. While I agree that this is a long way from the utility of our FITS reader, I think it's still a vaild use case.

@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2018

Oh pickle like.

@nabobalis nabobalis changed the title [WIP] Initial implementation of asdf schema and tag for map Initial implementation of asdf schema and tag for map Jan 24, 2018

@Cadair Cadair added this to the 1.0 milestone Jan 25, 2018

@Cadair Cadair removed this from High Priority Features in SunPy 0.9 Jan 25, 2018

@Cadair Cadair added this to High Priority Features in SunPy 1.0 Jan 25, 2018

@Cadair Cadair moved this from High Priority Features to Low Priority Features in SunPy 1.0 Jan 25, 2018

@Cadair Cadair force-pushed the Cadair:asdf_tags branch 2 times, most recently from 402e2a5 to e22301d Jan 25, 2018

@Cadair Cadair force-pushed the Cadair:asdf_tags branch from e22301d to 47aedd3 Mar 2, 2018

Cadair added some commits Sep 10, 2018

@Cadair Cadair force-pushed the Cadair:asdf_tags branch 2 times, most recently from fd5804f to cae5987 Dec 18, 2018

@Cadair Cadair force-pushed the Cadair:asdf_tags branch 3 times, most recently from 73f4659 to 436a68a Dec 19, 2018

@Cadair Cadair force-pushed the Cadair:asdf_tags branch from 436a68a to 60be3b0 Dec 20, 2018

Cadair added some commits Dec 21, 2018

@Cadair

This comment has been minimized.

Copy link
Member Author

commented Jan 3, 2019

The codecov failure is a lie.

@Cadair Cadair merged commit 87bf0b1 into sunpy:master Jan 3, 2019

10 of 11 checks passed

codecov/patch 41.3% of diff hit (target 75.2%)
Details
ci/circleci: 32bit Your tests passed on CircleCI!
Details
ci/circleci: egg-info-36 Your tests passed on CircleCI!
Details
ci/circleci: egg-info-37 Your tests passed on CircleCI!
Details
ci/circleci: figure-tests-36 Your tests passed on CircleCI!
Details
ci/circleci: html-docs Your tests passed on CircleCI!
Details
codecov/project 85.91% (+10.7%) compared to 3bdb581
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
giles Click details to preview the documentation build
Details
sunpy-bot All checks passed
sunpy.sunpy Build #20181221.3 succeeded
Details

@Cadair Cadair deleted the Cadair:asdf_tags branch Jan 3, 2019

@Cadair Cadair moved this from Low Priority Features to Finished in SunPy 1.0 Mar 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.