Auto-generate units stubs for wheels by mhvk · Pull Request #18573 · astropy/astropy

mhvk · 2025-08-30T18:58:29Z

This PR ensures that type stubs files are generated as part of building wheels (or in-place for editable installs).

OLD TEXT

This is by no means a finished product, but I wanted to see whether with our current setup it is possible to ensure the unit stubs get added to the wheels, and thought I would share the result so far.

It turns out it is possible, though (as @eerovaher noted elsewhere) at present it means requiring pyerfa as a build requirement (this problem is fairly easy to solve; EDIT: solved in bd9807e). A good bit is that it does properly make sure that the __pycache__, etc., files created while making the *.pyi files are not included.

~~Note that the scheme here does not work for editable installs; I've tried a bit to get that to work, and am sure it is possible, but didn't find an elegant solution for it.~~ EDIT: the scheme now also works for editable installs (b6ad186). In this case, the *.pyi files are stored directly in the project directory, and some __pycache__ appears too - but this should be fine as one will create those anyway upon using the editable install.

Anyway, mostly just to give everybody a sense of what's involved.

cc @eerovaher, @nstarman, @mikez

github-actions · 2025-08-30T18:58:44Z

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

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 instructions for rebase and squash.
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. Codestyle issues can be fixed by the bot.
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 this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
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.

github-actions · 2025-08-30T18:58:44Z

👋 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 · 2025-08-31T01:56:46Z

Thanks @mhvk!
As a side point, for those interested in an eventual upgrade to a newer build system like Meson, AFAIK this type of build customization is easily supported in those systems as well.

mhvk · 2025-08-31T12:24:49Z

Thanks. Your comment about meson/hatch in the other thread actually was what got me thinking that surely setuptools supports this kind of thing too. Of course, the annoyance here is that one is effectively bootstrapping -- we need at least part of astropy to be usable to generate the type stubs. But I don't see much else in astropy that would need similar treatment, so a bit of hackery here may be worthwhile.

mhvk · 2025-08-31T13:52:16Z

ping also @neutrinoceros, since he was interested in getting the typing working too.

mhvk · 2025-08-31T17:11:25Z

OK, I think this now works well enough, so marking it ready for review.

Thanks, @mikez, for getting this going! The script is now used for building the .pyi files automatically, so arguably is not directly needed any more, but it is what got this finally going.

mhvk · 2025-08-31T18:02:19Z

The reason for the skip-changelog-checks is that I edit the changelog from #18547 here.

mhvk · 2025-08-31T18:21:40Z


-def test_typestubs_for_units_script(tmp_path: Path, monkeypatch):
+def test_typestubs_installed():
+    spot_check_stubs(Path(u.__file__).parent)


Note that our tests cover editable installs:

astropy/.github/workflows/ci_workflows.yml

Lines 72 to 81 in 1ba18e8

- name: Python 3.12 in Parallel with all optional dependencies

linux: py312-test-alldeps-fitsio

libraries:

apt:

- language-pack-fr

- tzdata

- libbz2-dev

- libcfitsio-dev

toxargs: -v --develop

posargs: -n=4 --run-slow

eerovaher · 2025-09-01T16:18:24Z

There are three options worth considering for distributing the stub files for unit definitions.

Commit the stubs normally. This might be the simplest option. The main downside with this option is that stub files are quite large. In order to test the stub files with stubtest they have to contain an explicit __all__, so the stub files in Add stub files with unit definitions #17902, which do not contain an __all__, are roughly half the size of the testable stubs. For example, a proper astropy/units/si.pyi would be 83 kB. That being said, the blob Git would store that stub as would be compressed by a factor of 5. If we decide that a couple of tens of kilobytes is a fair price to pay then we don't have to implement any machinery for generating the stubs automatically. A script that the developers could run to regenerate the stubs would be good enough.
Generate the stubs automatically during installation. This is the approach this pull request attempts. The main advantage over 1) is that there is no need to commit the stubs, so their size is less of a concern. The main downside is that the machinery required to generate the stubs automatically is not trivial. However, if we are willing to accept the changes required in setup.py then this can be a very good option.
Maintain a separate stub-only package. I think in principle this is the least good of the three options, but in practice it might be appealing for astropy because it would allow astropy not to worry about the stubs at all. Generating and testing the stubs would be entirely the responsibility of astropy-stubs. Users would just have to install astropy together with the astropy-stubs package, and because dependencies can be listed in pyproject.toml then users might not even have to install astropy-stubs explicitly.

If we were to have a vote about this pull request then I would vote in favor of merging it, but if this proves to be too controversial then I don't intend to get involved in any arguments over this. The tradeoffs involved in creating a stub-only package are acceptable to me.

astrofrog · 2025-09-02T12:45:05Z

Just to make sure I understand, is the long term plan to just ship stubs for units or for other subpackages too?

mhvk · 2025-09-02T13:01:08Z

Just to make sure I understand, is the long term plan to just ship stubs for units or for other subpackages too?

Just the units. We need to do those because they are auto-generated, but at the moment at least there is nothing else in astropy that gets generated on the fly. (Even for the units, if I were to start from scratch, I would not create public unit instances, but always create with Unit(string).)

nstarman

IMO this looks great. I'm no expert in setup.py and defer detailed judgement of the install commands to those that are. To reiterate on a related note, there has been continued interest in following numpy, scipy et al away from setuptools and to e.g. Meson. With Meson, etc a very similar build pattern is supported, so this PR should not make any future transition more challenging.

nstarman · 2025-09-02T14:40:25Z

Delaying the erfa imports is the part I'm definitely least happy about, but can live with :).
I think fixing this is a challenging followup process that will likely involve refactoring units and constants to have a more integrated structure (even if the public presentation as two subpackages remains unchanged).
Anyway, that is best discussed elsewhere!

In follow-up, I'd like the helpers for both scipy and erfa to do all the helper definitions only when the functions are actually needed, i.e., they would be moved to _scipy and _erfa modules that get imported on demand.

The advantage is that in those delayed-import modules, one simply has all units available, so the local imports of those in helper functions would disappear -- hence, it is a cleanup that is good independent of what's here!

astrofrog · 2025-09-03T10:00:00Z

Just the units. We need to do those because they are auto-generated, but at the moment at least there is nothing else in astropy that gets generated on the fly. (Even for the units, if I were to start from scratch, I would not create public unit instances, but always create with Unit(string).)

Perhaps a wild idea but have we considered hard coding all the units in a Python file (or multiple files) and including a pre-commit script that can ensure this auto generated file is up to date? This would make the stub issue moot and also likely speed up astropy.units imports. Obviously not perfect, but worth considering?

mhvk · 2025-09-06T19:55:41Z

OK, I updated the PR, moving the functions that do the actual work from the script to a private module, and deleting the added documentation and what's-new. I left the changelog entry in place, as I think this is an important step worth mentioning, but am happy to remove that too if requested.

mhvk · 2025-09-06T19:58:35Z

p.s. I made a few light cleanups of the type-stub generating functions, but clearly more can be done. I suggest we leave that to follow-up, though. SImilarly, @eerovaher suggested the stubs themselves can be checked by stubtest - I feel that belongs in follow-up as well, ideally by someone who actually uses the type stubs.

mhvk · 2025-09-06T20:19:22Z

Failures are unrelated.

nstarman

LGTM. The existing tests haven't changed, only now stubs are always generated. This does increase the need for more thorough testing, but I agree this can be a followup.

mhvk · 2025-09-07T13:24:59Z

Last (force) push removed the mention of the script in pyproject.toml, which I had forgotten.

eerovaher · 2025-09-08T16:52:33Z

+        elif name == "enable":
+            variable_lines.append("enable() -> _UnitContext: ...")


Because of this addition astropy/units/imperial.pyi contains invalid syntax, so it is not even a valid stub file anymore. I don't know if you tested the stubs or not. If you did and the tests approved them despite invalid syntax then the tests are worthless. I suggest you don't try to make any changes to the contents of the generated stubs in this pull request.

At least some files that stubs are generated for do include enable in their __all__, and my intent was to try to ensure the stub files were complete (since I think otherwise enable would just be untyped). To test this, I changed the code to raise RuntimeError below.

But if what I add here is wrong, then clearly my lack of familiarity with typing is biting me here, and I should just leave it for follow-up. What do you suggest? Just skipping only enable, or just removing the RuntimeError below.

Alternatively, if you can give the correct typing statement, I can do that too (or better still, just push to my branch).

p.s. Unfortunately, with teaching started, I do not have the time to start learning how to check typing -- as you know, typing is not something I particularly care about, I really just wanted to have a conclusion to the discussion about type stubs...

my intent was to try to ensure the stub files were complete

If the output contains syntax errors then it's not a stub file at all.

To test this, I changed the code to raise RuntimeError below.

Leave testing the stubs to stubtest.

What do you suggest? Just skipping only enable

This pull request should be concerned with implementing the mechanism that generates the stubs when astropy is built and cleaning up the documentation. It should not be concerned with testing the stubs or with the contents of the generated stubs.

or just removing the RuntimeError below.

It is very bad if a mistake in generating the stubs can cause astropy installation to fail, so having the script throw a RuntimeError not a good idea at all.

Alternatively, if you can give the correct typing statement, I can do that too (or better still, just push to my branch).

Let's just merge the mechanism for automatically generating the stubs. It's simpler to deal with the stubs themselves in a separate pull request.

OK, I removed it; I'll leave it for typing afficionados to add proper checking (and then add enable).

eerovaher · 2025-09-08T16:53:09Z

So developers know we're a step further and they can start helping out / using it. If others agree that this changelog entry is not necessary, I'm happy to remove it, but to me it seems obviously useful.

Downstream packages cannot use astropy annotations for type checking their code unless we declare our code to be type-checkable with a py.typed marker file. So bringing up annotations in our change log before we do is just a source of confusion. And when it comes to e.g. IDE completions then any development environment worth using will be able to find the stubs automatically. So there is nothing to announce.

As I wrote, my message was aimed at people who might think the time was now right to help improve the typing. But clearly it didn't get through to you that way, and I don't really care, so the easiest solution is to delete it.

The change log is for the users. If you want to invite developers to work on type annotations then you should open an issue on GitHub, or perhaps write to the mailing list.

This moves the previous script to a new file in typing_utils and adjusts it so that it can be called from setup.py on installation (in regular or editable mode). Test are adjusted accordingly. Since typing is at present not directly useful, and IDEs will find these stubs automatically, no announcement of this is made in the changelog or documentation.

mhvk · 2025-09-09T00:56:58Z

OK, I pushed a minimal version that is hopefully OK with everyone. Shrunk to two commits so there are no reversions internally.

neutrinoceros

I finally did a full-on review, and while I did try catching up with the existing conversations, I may have overlooked some parts, so I apologise if any of my questions/suggestions were already resolved.

To reiterate on a related note, there has been continued interest in following numpy, scipy et al away from setuptools and to e.g. Meson. With Meson, etc a very similar build pattern is supported, so this PR should not make any future transition more challenging.

as a side note: @astrofrog and myself are currently working on an APE that may provide some help with any hypothetical transition to more modern build systems 🤞🏻

neutrinoceros · 2025-09-09T11:46:08Z

+# been completed, so we import units locally.
+# We also avoid importing erfa up front since otherwise type stub generation
+# in the build process can only run with erfa as a build requirement.
+# TODO: solve both of the above by defining inside get_erfa_helpers?


To be clear, are you planning to implement these TODOs are part of this PR, or as a follow up ?

Definitely for future follow-up (would want to do scipy similarly at the same time).

neutrinoceros · 2025-09-09T12:08:15Z

-    typestubs_for_units.main()
-
+def test_typestubs_installed():
+    path = Path(u.__file__).parent


Since this looks like the only use of astropy.units in this test module, and in order to isolate it as much as possible from potential import side effects, I would recommend using importless-introspection instead, such as

from importlib.util import find_spec if (units_modspec := find_spec("astropy.units")) is None: # this is so typecheckers can confidently narrow the type of `units_modspec` in the following raise AssertionError() path = Path(units_modspec.origin)

I don't think in this context it matters much - these tests are run after installation and as part of the a pytest run, where surely other tests have already imported astropy.units.

Also, @eerovaher suggested that after this PR, the testing should be done using standard type testing, so this will likely be a temporary solution anyway.

There is nothing to gain from updating this file any further.

neutrinoceros · 2025-09-09T12:12:35Z

            "numpy>=2.0.0, <3",
-            "extension-helpers>=1.4,<2"]
+            "extension-helpers>=1.4,<2",
+            "pyerfa>=2.0.1.1"]


while we're at it, can we format it in a way that we don't have to loose useful git blame info next time we want to add a line without actually changing this one ?

Suggested change

"pyerfa>=2.0.1.1"]

"pyerfa>=2.0.1.1",

]

I can do that if other changes need to be made.

if you're worried about wasting CI resources, you can always include [skip ci] in the commit message

neutrinoceros · 2025-09-09T12:18:16Z

+from pathlib import Path
+
 from setuptools import setup
+from setuptools.command.editable_wheel import editable_wheel


I could hardly find any documentation for this class, so my question is this:
how did you learn about it and how do you know that we got every case covered ?

Short answer: I don't. For the regular installation, there were quite a few clues on-line, but for the actual implementation I ended up looking through the setuptools source code.

Aside: from my experience with meson so far, the learning curve is no less steep for that. Anything unusual and quickly you need to look at source code. The meson.build files remain rather unreadable to me (at least for numpy, which of course is not the easiest package).

p.s. Just a bit more positively, for regular installs I'm sure what I'm doing is OK. I am not sure that my way to cover editable installs covers every possibility to get an editable install, but think I'm OK too, since I'm on the path that the relevant PEP wants.

Sorry I don't know what PEP you're hinting at.

https://peps.python.org/pep-0660/

neutrinoceros

all my questions and suggestions were answered, so approving now !

mhvk · 2025-09-11T13:42:56Z

Thanks! With two approvals, let's just get this in.

pllim · 2025-09-11T15:42:29Z

Didn't we fantasize about getting rid of setup.py altogether at one point? I guess it is not happening now...

Indeed, not if we keep using setuptools. But at least setup.py now is fairly simple, with concrete things that it does.

@pllim Yes, It would be nice to remove. But @mhvk is right that we can't do so while we use setuptools. 🤞...

whatever build backend we migrate to if/when we get away from setuptools, we'll need to migrate this bit of code generation as well

to add build for Windows ARM64. Also backport the ignore added in astropy#18573 so devs do not accidentally check in pyi files when switching branches locally.

to add build for Windows ARM64. Also backport the ignore added in #18573 so devs do not accidentally check in pyi files when switching branches locally.

mhvk added this to the v7.2.0 milestone Aug 30, 2025

mhvk added units installation typing related to type annotations labels Aug 30, 2025

mhvk force-pushed the units-auto-generate-unit-stubs branch from b6ad186 to b881407 Compare August 31, 2025 17:08

mhvk marked this pull request as ready for review August 31, 2025 17:09

mhvk requested review from eerovaher, neutrinoceros and nstarman August 31, 2025 17:09

mhvk added the Enhancement label Aug 31, 2025

mhvk force-pushed the units-auto-generate-unit-stubs branch 3 times, most recently from 9451907 to 470dd37 Compare August 31, 2025 17:57

mhvk added the skip-changelog-checks Tells bot to skip changlog checks label Aug 31, 2025

mhvk commented Aug 31, 2025

View reviewed changes

This was referenced Aug 31, 2025

Possibility of Generating pyi Files astropy/extension-helpers#129

Closed

RFC: Automatic generation of type stubs - subclassing install? pypa/setuptools#5078

Open

nstarman approved these changes Sep 2, 2025

View reviewed changes

nstarman mentioned this pull request Sep 2, 2025

units: alias Unit as U for concise construction #18582

Open

mhvk force-pushed the units-auto-generate-unit-stubs branch from be229f3 to a2c6fc4 Compare September 6, 2025 19:55

mhvk commented Sep 6, 2025

View reviewed changes

Comment thread astropy/units/imperial.py Resolved

nstarman approved these changes Sep 7, 2025

View reviewed changes

mhvk force-pushed the units-auto-generate-unit-stubs branch from a2c6fc4 to 29e709d Compare September 7, 2025 13:24

eerovaher reviewed Sep 8, 2025

View reviewed changes

mhvk mentioned this pull request Sep 9, 2025

Ensure imperial.__all__ also contains 'enable' #18604

Merged

mhvk added 2 commits September 8, 2025 20:44

MAINT: Ensure pyerfa is not needed for stubs building

137efc2

mhvk force-pushed the units-auto-generate-unit-stubs branch from 29e709d to 842c9ca Compare September 9, 2025 00:50

neutrinoceros reviewed Sep 9, 2025

View reviewed changes

This comment was marked as duplicate.

Sign in to view

WilliamJamieson mentioned this pull request Sep 10, 2025

Could type-stubs be auto generated for the nodes? spacetelescope/rad#698

Open

neutrinoceros approved these changes Sep 11, 2025

View reviewed changes

mhvk merged commit cde31aa into astropy:main Sep 11, 2025

mhvk deleted the units-auto-generate-unit-stubs branch September 11, 2025 13:43

pllim reviewed Sep 11, 2025

View reviewed changes

neutrinoceros mentioned this pull request Sep 18, 2025

WHL: wheels failing to build for macOS x86_64 #18621

Closed

neutrinoceros mentioned this pull request Oct 22, 2025

WHL: consistently use build as the build frontend #18421

Merged

1 task

pllim added a commit to pllim/astropy that referenced this pull request Oct 28, 2025

Backport of astropy#18786 onto v7.1.x

79758e5

to add build for Windows ARM64. Also backport the ignore added in astropy#18573 so devs do not accidentally check in pyi files when switching branches locally.

pllim mentioned this pull request Oct 28, 2025

Backport PR #18786 on branch v7.1.x (Add arm64 windows support) #18793

Merged

1 task

pllim added a commit that referenced this pull request Oct 28, 2025

Backport of #18786 onto v7.1.x (#18793)

7118415

to add build for Windows ARM64. Also backport the ignore added in #18573 so devs do not accidentally check in pyi files when switching branches locally.

neutrinoceros mentioned this pull request Nov 19, 2025

EXP: revert #18164 #18971

Closed

1 task

pllim mentioned this pull request Mar 16, 2026

BLD/DEP: drop dangling build-time dependency on pyerfa #19430

Closed

1 task

	- name: Python 3.12 in Parallel with all optional dependencies
	linux: py312-test-alldeps-fitsio
	libraries:
	apt:
	- language-pack-fr
	- tzdata
	- libbz2-dev
	- libcfitsio-dev
	toxargs: -v --develop
	posargs: -n=4 --run-slow

		elif name == "enable":
		variable_lines.append("enable() -> _UnitContext: ...")

Conversation

mhvk commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 30, 2025

Uh oh!

github-actions Bot commented Aug 30, 2025

Uh oh!

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

Uh oh!

Uh oh!

mhvk commented Aug 31, 2025

Uh oh!

mhvk commented Aug 31, 2025

Uh oh!

mhvk commented Aug 31, 2025

Uh oh!

mhvk commented Aug 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eerovaher commented Sep 1, 2025

Uh oh!

astrofrog commented Sep 2, 2025

Uh oh!

mhvk commented Sep 2, 2025

Uh oh!

nstarman left a comment

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!

astrofrog commented Sep 3, 2025

Uh oh!

mhvk commented Sep 6, 2025

Uh oh!

Uh oh!

mhvk commented Sep 6, 2025

Uh oh!

mhvk commented Sep 6, 2025

Uh oh!

nstarman left a comment

Choose a reason for hiding this comment

Uh oh!

mhvk commented Sep 7, 2025

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!

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!

mhvk commented Sep 9, 2025

Uh oh!

neutrinoceros left a comment

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!

Choose a reason for hiding this comment

mhvk commented Aug 30, 2025 •

edited

Loading

nstarman commented Aug 31, 2025 •

edited

Loading