cosmo: make public io submodule by nstarman · Pull Request #14982 · astropy/astropy

nstarman · 2023-06-25T16:37:31Z

In a previous PR (#14842) I made the private io module actually private.
This PR:

introduces a public io module , which is much more carefully managed.
- moves the contents of _io to io/_builtin.
moves the connect.py file to io as a private module, but publicly importing it's classes and attributes.
- deprecates the connect.py module for later removal.
modifies the docs, though most of the content was introduced in Cosmo io docs #15090

TODO:

docs : see https://astropy--14982.org.readthedocs.build/en/14982/cosmology/io/index.html
changelog
whatsnew

github-actions · 2023-06-25T16:38:05Z

github-actions · 2023-06-25T16:38:10Z

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

nstarman · 2023-08-13T19:00:03Z

ping @mhvk, @eerovaher. If you have time to review this. I'm trying to consolidate and clean up the I/O structure. Most of this is very straightforward, just moving files around and adding __getattr__ to keep the API identical and enable a deprecation period.

mhvk · 2023-08-14T15:39:59Z

See #15169 for discussion of how one should indicate what is private.

nstarman · 2023-08-14T17:12:14Z

@mhvk, I think this PR can proceed as all the content currently lives in _io which is already private and nothing changes on this front in the move to io/_builtin. A resolution to #15159 and astropy/astropy-APEs#85 might require a new PR to rename _builtin to builtin but also to the current incarnation _io -> io.

nstarman · 2023-10-16T15:51:30Z

Thanks @eerovaher for the review.

nstarman · 2024-04-16T21:16:49Z

@mhvk @eerovaher @WilliamJamieson, I think this PR can proceed as all the content currently lives in _io which is already private and nothing changes on this front in the move to io/_builtin. A resolution to #15159 and astropy/astropy-APEs#85 might require a new PR to rename _builtin to builtin but also to the current incarnation _io -> io.

eerovaher · 2024-04-18T18:50:26Z

    >>> cosmo
-    FlatLambdaCDM(name="Planck18", H0=67.66 km / (Mpc s), Om0=0.30966,
-                  Tcmb0=2.7255 K, Neff=3.046, m_nu=[0. 0. 0.06] eV, Ob0=0.04897)
+    FlatLambdaCDM(name='Planck18', H0=<Quantity 67.66 km / (Mpc s)>, Om0=0.30966,
+                  Tcmb0=<Quantity 2.7255 K>, Neff=3.046,
+                  m_nu=<Quantity [0.  , 0.  , 0.06] eV>, Ob0=0.04897)


Renaming modules should not cause FlatLambdaCDM.__repr__() to produce different output, and it looks like pytest agrees with me. This is a large pull request, I'd rather not review it until it has been cleaned up.

I don't think these doctests were being tested! Trying to find the exact change that made them start being tested.

I'm trying to identify the change that caused all these docstrings to start being tested. But regardless of the cause, I can confirm that the old docstrings are wrong, the new ones are correct, and that the docstrings are now being tested.

The output here depends on the Python version, see

astropy/pyproject.toml

Lines 204 to 207 in dde34be

doctest_subpackage_requires = [

"astropy/cosmology/_io/mapping.py = python<3.12", # PYTHON_LT_3_12 (PR 14784)

"astropy/cosmology/_io/row.py = python<3.12", # PYTHON_LT_3_12 (PR 14784)

"astropy/cosmology/_io/table.py = python<3.12", # PYTHON_LT_3_12 (PR 14784)

There's no need to update the docstrings in this pull request.

Ah. And since _io is now io...
I can separate this into 2 PRs, first for the re-org. Then for fixing the docstrings.

Docstring difference looks like repr vs str on the quantities. Not sure repr is better here.

'>>>' calls repr. We would have to do print for everything.

Maybe Quantity's repr should be updated to call repr on its contents and enable eval(repr(q))

I don't like that eval(repr(q)) doesn't work, but admittedly for arrays this is always going to be a problem (at least large ones). And ndarray doesn't produce a repr that can be evaluated either... This is why I was wondering if it really made sense to use repr(q) in cosmology.

And ndarray doesn't produce a repr that can be evaluated either...

But other array types can! Maybe Quantity 2.0 is a good opportunity to enable the repr behavior, to support all the other array types.

@eerovaher, done! removed the doc edits to mapping, row, and table.

eerovaher

I have a few remarks, but I might have a few more when I've had a look at the rendered docs.

eerovaher · 2024-04-19T19:56:12Z

+:mod:`~astropy.cosmology.connect` module. It is intended that more public I/O
+functionality will be added to the :mod:`~astropy.cosmology.io` module in the future.


Not sure if it's a good idea to make such public promises.

I want to make much of the contents of _builtin that are related to converting between formats publicly accessible. I don't think there's a good way to type hint to(from)_format and I also want to both support a more functional approach to I/O. Also I would love to enable entry points so 3rd party packages can better register I/O formats. All it takes is copious free time to do...

github-actions · 2024-09-17T05:35:27Z

Hi humans 👋 - this pull request hasn't had any new commits for approximately 4 months. I plan to close this in 30 days if the pull request doesn't have any new commits by then.

In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.

If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock.

If you believe I commented on this pull request incorrectly, please report this here.

Signed-off-by: Nathaniel Starkman <[email protected]> Signed-off-by: nstarman <[email protected]>

Co-authored-by: Eero Vaher <[email protected]> Signed-off-by: nstarman <[email protected]> Update docs/whatsnew/7.0.rst Co-authored-by: Eero Vaher <[email protected]>

Signed-off-by: nstarman <[email protected]>

nstarman added this to the v6.0 milestone Jun 25, 2023

github-actions Bot added the cosmology label Jun 25, 2023

nstarman force-pushed the cosmo-move-connect branch 5 times, most recently from 2cdf9e9 to 2a2ad35 Compare July 25, 2023 04:54

nstarman force-pushed the cosmo-move-connect branch 7 times, most recently from f65c78b to 8b79cdd Compare August 7, 2023 19:09

nstarman marked this pull request as ready for review August 7, 2023 19:11

nstarman requested a review from a team as a code owner August 7, 2023 19:11

nstarman requested review from WilliamJamieson, eerovaher and mhvk and removed request for a team August 7, 2023 19:11

nstarman force-pushed the cosmo-move-connect branch from 8b79cdd to b44e240 Compare August 13, 2023 18:55

mhvk mentioned this pull request Aug 14, 2023

How to indicate which sub-sub-modules are private? #15169

Open

nstarman force-pushed the cosmo-move-connect branch from b44e240 to aa857b4 Compare August 23, 2023 02:14

nstarman force-pushed the cosmo-move-connect branch 2 times, most recently from 9d4c2a8 to b9025ae Compare September 1, 2023 19:19

nstarman force-pushed the cosmo-move-connect branch from 24af642 to 5ebc11d Compare October 16, 2023 15:50

nstarman requested a review from eerovaher October 16, 2023 15:51

nstarman force-pushed the cosmo-move-connect branch from 5ebc11d to 42e1183 Compare October 24, 2023 04:22

saimn modified the milestones: v6.0, v6.1 Oct 28, 2023

nstarman force-pushed the cosmo-move-connect branch 2 times, most recently from 8072a97 to 2adb55b Compare January 14, 2024 00:10

astrofrog modified the milestones: v6.1.0, v7.0.0 Apr 4, 2024

nstarman force-pushed the cosmo-move-connect branch 2 times, most recently from 69b2198 to 93317c6 Compare April 16, 2024 21:10

nstarman force-pushed the cosmo-move-connect branch from 0594a0a to a7ccc4f Compare April 16, 2024 22:58

eerovaher reviewed Apr 18, 2024

View reviewed changes

nstarman force-pushed the cosmo-move-connect branch from a7ccc4f to 322df98 Compare April 19, 2024 19:10

eerovaher reviewed Apr 19, 2024

View reviewed changes

github-actions Bot added the Close? Tell stale bot that this issue/PR is stale label Sep 17, 2024

nstarman force-pushed the cosmo-move-connect branch 2 times, most recently from 11f6067 to 8cf4f4c Compare October 9, 2024 20:52

nstarman removed the Close? Tell stale bot that this issue/PR is stale label Oct 9, 2024

nstarman and others added 3 commits October 10, 2024 11:36

Make a public IO module

6acd677

Signed-off-by: Nathaniel Starkman <[email protected]> Signed-off-by: nstarman <[email protected]>

cleanup

30fa979

Co-authored-by: Eero Vaher <[email protected]> Signed-off-by: nstarman <[email protected]> Update docs/whatsnew/7.0.rst Co-authored-by: Eero Vaher <[email protected]>

docs: move typing definitions

1d444ac

Signed-off-by: nstarman <[email protected]>

nstarman force-pushed the cosmo-move-connect branch from 8cf4f4c to 1d444ac Compare October 10, 2024 15:38

nstarman requested a review from eerovaher October 10, 2024 17:04

saimn modified the milestones: v7.0.0, v7.1.0 Oct 26, 2024

nstarman closed this Dec 12, 2024

	doctest_subpackage_requires = [
	"astropy/cosmology/_io/mapping.py = python<3.12", # PYTHON_LT_3_12 (PR 14784)
	"astropy/cosmology/_io/row.py = python<3.12", # PYTHON_LT_3_12 (PR 14784)
	"astropy/cosmology/_io/table.py = python<3.12", # PYTHON_LT_3_12 (PR 14784)

		:mod:`~astropy.cosmology.connect` module. It is intended that more public I/O
		functionality will be added to the :mod:`~astropy.cosmology.io` module in the future.

Uh oh!

Conversation

nstarman commented Jun 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 25, 2023

Uh oh!

github-actions Bot commented Jun 25, 2023

Uh oh!

nstarman commented Aug 13, 2023

Uh oh!

mhvk commented Aug 14, 2023

Uh oh!

nstarman commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nstarman commented Oct 16, 2023

Uh oh!

nstarman commented Apr 16, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nstarman Apr 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nstarman Apr 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eerovaher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nstarman commented Jun 25, 2023 •

edited

Loading

nstarman commented Aug 14, 2023 •

edited

Loading

nstarman Apr 18, 2024 •

edited

Loading

nstarman Apr 18, 2024 •

edited

Loading