klayout
klayout copied to clipboard
`scripts/make_stubs.sh` is not run automatically when releasing new pypi versions
I think when a new pypi package is published, the intention would be to update/create the python stub files. As far as I can tell, it would make sense to run these when/before running the wheel build(s). It seems thought that currently these files are tracked in the repository and don't get automatically updated.
Would it make sense to automate this? I am happy to make a PR if anyone can point me to where to do it. I tested it locally and I only could get it to run if the GUI has been built previously. But I am sure there is a better way.
@thomaslima
(I noticed this when mypy was complaining that new API points aren't available, when they are there in the official docs and the local docs built into KLayout itself)
Hey @sebastian-goeldi
Hallo Sebastian,
Good catch! The stubs can be generated without building the GUI version. Mathias made a nice script make_stubs.sh
that aggregates all stubgen commands, but it looks like it's used on a standard compilation. I just ran it successfully by commenting line 26, disabling the pymod
check. Indeed I noticed the pyi files had some minor changes. I'm thinking the solution would be an auto-PR triggered by GitHub when it detect changes in the pyi files.
Would you mind sharing what issue your mypy complained about? Did the new stubs fix it? I wanted to make sure that it isn't a bug in our stubgen script.
Thanks!
Hey Thomas,
Sorry, my wording might have been off. Mypy complained because I was using the new API (the metainfos ;)) and mypy thought that those functionalities don't exist. After compiling klayout and running the make_stubs.sh
script and then installing klayout with python -m pip install /path/to/klayout/repo/folder
, it worked (no more mypy errors). That's why I was pretty sure that the make_stub.sh
script wasn't run automatically by CICD of github. The generation is correct and worked great after manually updating them.
I think the ideal solution imo would be to actually run this on every commit where the python wheel is created and actually exclude them from being added to the git repo (automatically) via .gitignore
. This way you also make sure that there aren't old artifacts lingering in the stub files. But I am not sure how much of a performance hit on CICD this would entail
I understood -- thanks for checking the stubgen script still works. You raise an interesting proposal. But there are issues with it. We need to check with Matthias because the wheels that are uploaded are not simply coming from GitHub actions. Sometimes he builds wheels manually then uploads (for Windows I believe). Not to mention this make_stubs would not work in Windows...
I actually like manually diffing the pyi stubs to see what functionality is coming through between versions. To mitigate this, in #1391 I am simply detecting changes to the pyi's and failing the GitHub action unless the new stubs are pushed. I think this approach, coupled with some automated PR, should help a lot.
The wheels are generated by Azure and GitHub jobs but I like to have the stubs in the release tag too. That means the stubs have to be created before that.
I have a release checklist and I will put running the make_stubs script on that checklist. I did not pay attention to that in the past, but I will try to do so in the future. There is a lot more related to a release such as creating the release notes so that is little overhead.
I prefer that solution over some bot that needs maintenance or generates out-of-band commits.
I closed the PR #1391 containing an automated stub generation bot. You can revisit it in the future, if you'd like. Honestly I am surprised the stubs are changing this frequently -- I thought they would be very stable over time.