Skip to content

Added tags to serialize all functional models. #10028

Merged
nden merged 5 commits intoastropy:masterfrom
cshanahan1:serialize_functional_models
Apr 29, 2020
Merged

Added tags to serialize all functional models. #10028
nden merged 5 commits intoastropy:masterfrom
cshanahan1:serialize_functional_models

Conversation

@cshanahan1
Copy link
Copy Markdown
Contributor

Added functional_models.py in io/misc/asdf/tags/transform with new tags for each of the functional models (with the exception of a few that already existed in polynomial and basic), as well as tests for each.

Tests pass for me locally, but will not here unless the new schemas in asdf-standard are pointed to.

closes #9959 with redundant changes.

@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Mar 10, 2020

Hello @shannnnnyyy 👋! It looks like you've made some changes in your pull request, so I've checked the code again for style.

Line 11:55: W291 trailing whitespace
Line 12:63: W291 trailing whitespace

Comment last updated at 2020-04-29 05:41:03 UTC

@astropy-bot astropy-bot bot added io.misc zzz 💤 io.misc.asdf archived: Go to asdf-astropy repo labels Mar 10, 2020
@pllim pllim requested a review from nden March 10, 2020 17:36
@pllim pllim added the modeling label Mar 10, 2020
@pllim pllim added this to the v4.1 milestone Mar 10, 2020
@cshanahan1 cshanahan1 force-pushed the serialize_functional_models branch 3 times, most recently from b9779ed to e885b08 Compare March 10, 2020 17:45
@cshanahan1
Copy link
Copy Markdown
Contributor Author

uh oh, i made some sort of git mistake and it reverted the files back to a way old commit. let me see if i can fix this...

@cshanahan1 cshanahan1 force-pushed the serialize_functional_models branch 3 times, most recently from 26d836d to b525b0a Compare March 18, 2020 04:08
@nden
Copy link
Copy Markdown
Contributor

nden commented Mar 31, 2020

This looks great!
I'm not approving because it is waiting on a release of asdf in order to be merged.
The duplicate change log needs to be removed but perhaps that's taken care of already.

@bsipocz
Copy link
Copy Markdown
Member

bsipocz commented Mar 31, 2020

This will also need a rebase to pick up fixed for the unrelated CI failures.

@cshanahan1 cshanahan1 force-pushed the serialize_functional_models branch 2 times, most recently from ca1f156 to f581eba Compare April 6, 2020 05:34
@cshanahan1 cshanahan1 force-pushed the serialize_functional_models branch 2 times, most recently from e5e1072 to f3b900b Compare April 21, 2020 23:43
astmodels.SphericalRotationSequence([1.2, 2.3, 3.4, .3], 'xyzy'),
custom_and_analytical_inverse(),
]
astmodels.AiryDisk2D(10., 0.5, 1.5),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

People can specify parameter values as keyword arguments, right? It might be a good idea to also construct them that way, so we can protect that aspect of the public interface.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah - the existing tests didn't do it this way so I just stuck with supplying the arguments like that, but I can change them all to use keyword arguments.

@nden
Copy link
Copy Markdown
Contributor

nden commented Apr 27, 2020

This also needs a rebase to pick up the latest version of asdf.

@cshanahan1 cshanahan1 force-pushed the serialize_functional_models branch from bb14e26 to 22e16c7 Compare April 29, 2020 05:03
Copy link
Copy Markdown
Contributor

@nden nden left a comment

Choose a reason for hiding this comment

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

Thanks @shannnnnyyy !

@nden nden merged commit 2c34c5a into astropy:master Apr 29, 2020
@cshanahan1 cshanahan1 deleted the serialize_functional_models branch April 30, 2020 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

io.misc modeling zzz 💤 io.misc.asdf archived: Go to asdf-astropy repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants