Skip to content

cosmo: improve mapping io and tests - #12209

Merged
nstarman merged 1 commit into
astropy:mainfrom
nstarman:cosmo_improve_mapping
Oct 26, 2021
Merged

nstarman merged 1 commit into
astropy:mainfrom
nstarman:cosmo_improve_mapping

Conversation

@nstarman

@nstarman nstarman commented Sep 28, 2021 •

Copy link
Copy Markdown
Member

Cosmology instance conversion to dict now accepts keyword argument cls to
determine dict type, e.g. OrderedDict. This argument is largely driven by to/from_table (#12213) wherein __astropy_table__ allows for a cls argument. Implementing that in to_table meant doing so here as well. And I think it's the better for it.

Also the tests are much improved.
I think the partial information test is impossible to generalize, but this new format of test class inheritance means each format can provide a partial-information test that will be used to test the to/from_X function as well as Cosmology.to/from_format() for each Cosmology subclass.

Signed-off-by: Nathaniel Starkman (@nstarman) [email protected]

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the Extra CI label.
  • Is a change log needed? If yes, did the change log check pass? If no, add the no-changelog-entry-needed label. If this is a manual backport, use the skip-changelog-checks label unless special changelog handling is necessary.
  • Is a milestone set? Milestone must be set but astropy-bot check might be missing; do not let the green checkmark fool you.
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate backport-X.Y.x label(s) before merge.

@nstarman nstarman added this to the v5.0 milestone Sep 28, 2021
@pep8speaks

pep8speaks commented Sep 28, 2021 •

Copy link
Copy Markdown

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

There are no PEP8 style issues with this pull request - thanks! 🎉

Comment last updated at 2021-10-22 19:24:50 UTC

@nstarman
nstarman force-pushed the cosmo_improve_mapping branch from b6fb539 to 714c8a1 Compare September 28, 2021 18:53
@nstarman nstarman changed the title improve mapping io and tests cosmo: improve mapping io and tests Sep 28, 2021
@nstarman
nstarman force-pushed the cosmo_improve_mapping branch 3 times, most recently from 37b34fb to 69a5205 Compare September 29, 2021 03:14
@nstarman nstarman mentioned this pull request Sep 29, 2021
9 tasks done
@nstarman
nstarman requested a review from mhvk September 29, 2021 05:45
@nstarman
nstarman requested a review from adrn October 18, 2021 16:36
@nstarman nstarman mentioned this pull request Oct 22, 2021
5 of 9 tasks
Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>
@nstarman
nstarman force-pushed the cosmo_improve_mapping branch from 69a5205 to 031b1b7 Compare October 22, 2021 19:24

@taldcroft taldcroft left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems straightforward, LGTM.

@nstarman
nstarman merged commit 3353a7d into astropy:main Oct 26, 2021
@nstarman
nstarman deleted the cosmo_improve_mapping branch October 26, 2021 16:20
@nstarman

Copy link
Copy Markdown
Member Author

Thanks @taldcroft for the quick review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants