A roadmap for package infrastructure without astropy-helpers #52
Conversation
astrofrog
force-pushed
the
ape-astropy-helpers
branch
from
20b4930 to
ecb943c
Compare
APE17.rst
Outdated
| such as the core astropy package which have all their tags on branches (at least | ||
| in recent years), we will need to add a ‘developer’ tag on ``master`` straight | ||
| after branching, e.g. after creating a ``v4.0.x`` branch we should tag the next | ||
| commit on ``master`` as ``v4.1.dev``. |
There was a problem hiding this comment.
This example is quite confusing, probably more helpful if only have the pointer to the docs.
There was a problem hiding this comment.
It seems like this should be noted somewhere, though, or it will be forgotten in the future. Maybe part of release instructions?
There was a problem hiding this comment.
It also seems a bit problematic because it means affiliated packages would have to start doing this. And many affiliated packages do not have standardized release procedures/instructions...
There was a problem hiding this comment.
No I don't think affiliated packages would need to do this. This issue only occurs for packages that have a workflow where they tag releases in branches, but in my experience most affiliated packages don't do release branches.
There was a problem hiding this comment.
I think we should leave this in the APE - the APE is not documentation but just a technical document for the project, and I think it's important to note this here. But I've added a sentence that says
In any case, the documentation about how to release the core package as well astropy-affiliated packages will need to be updated to reflect the use of setuptools_scm.
and I've added links to the pages that need to be updated.
There was a problem hiding this comment.
We are including instructions about how to use setuptools_scm with release branches in an upcoming packaging guide.
APE17.rst
Outdated
| such as the core astropy package which have all their tags on branches (at least | ||
| in recent years), we will need to add a ‘developer’ tag on ``master`` straight | ||
| after branching, e.g. after creating a ``v4.0.x`` branch we should tag the next | ||
| commit on ``master`` as ``v4.1.dev``. |
There was a problem hiding this comment.
It seems like this should be noted somewhere, though, or it will be forgotten in the future. Maybe part of release instructions?
|
+1 since it simplifies packaging. |
|
+1 I also note that of the 400 packages that use astropy-helpers, I'm probably responsible for 8, only 2 of which are active and one 1 of which is not pure-python. (I'll try to remember to delete experiemental remaining packages from github ...) |
|
Another 👍 though like @bsipocz I use |
APE17.rst
Outdated
| Astropy-helpers provides a way for developers to use ``setup_package.py`` files | ||
| throughout a package to define extensions and definitions for external | ||
| libraries. This is one of the only parts of astropy-helpers which we think it | ||
| makes sense to preserve, and we argue that it is so general that it should be |
There was a problem hiding this comment.
Does it though? Won't defining package data and so on be clearer if they all happen in the root level at setup.cfg or something? There should be only one obvious way to do things?
| | submodule | | | ||
| +-------------------------------------+-------------------------------------+ | ||
|
|
||
| We propose that a new package called extension-helpers be created starting from |
There was a problem hiding this comment.
I am not convinced yet another package is necessary. Instead, we can show them how things should be done "natively" over at template?
There was a problem hiding this comment.
I don't quite understand how this would work as @pllim is proposing. Then wouldn't we have to push updates to all users of this stuff if e.g. we need to update the OpenMP stuff?
There was a problem hiding this comment.
Our draft packaging guide shows both how to define extensions natively for simple cases and with extension-helpers:
https://oa-packaging-guide-preview.readthedocs.io/en/latest/extensions.html
For astropy, it's definitely the case that we have enough extensions that using extension-helpers would be a lot simpler than defining everything in the setup.py file, and the auto-detection of cython files is also a nice feature.
@pllim - are you satisfied with this or do you have specific wording recommendations for the APE?
There was a problem hiding this comment.
I think for packages that have one or two C extensions might not need to use extension_helpers as the standard ways of registering extensions would be sufficient. Certainly for core I think extensions_helpers would simplify things enough to warrant it's existence.
There was a problem hiding this comment.
Okay. I think I understand now. Thanks!
astrofrog
force-pushed
the
ape-astropy-helpers
branch
from
5bde343 to
c53050c
Compare
|
Following some discussions at the Astropy Coordination Meeting (mostly with @astrofrog and myself), I've identified a few things to add to this APE (or possibly say "out of scope for the APE, but should do in the implementation stage"):
|
|
@eteq - I think the following section should now address all your points: Please check whenever you can and confirm whether you agree! |
astrofrog
force-pushed
the
ape-astropy-helpers
branch
from
646bfa3 to
a62fe16
Compare
|
The new Timeline for changes section looks fine to me, so approving this. In the second paragraph there are a few instances of the word "should" that might be changed to "shall" or "will" (e.g. " the migration guide should be advertised widely to all maintainers") to convey a more definitive requirement, but there is no doubt that these things will happen so this change is optional to me. |
APE17.rst
Outdated
| <http://docs.astropy.org/en/stable/development/releasing.html>`_ as well as | ||
| `packages using the astropy package template | ||
| <http://docs.astropy.org/en/stable/development/astropy-package-template.html#releasing-a-python-package>`_ | ||
| will need to be updated to reflect the use of setuptools_scm. |
There was a problem hiding this comment.
I think the template needs to be updated for quite a bit more than just setuptools_scm, right? That probably should be stated explicitly as a mandatory part of the process.
There was a problem hiding this comment.
I guess it's in the "timeline" section, although it's not clear exactly what makes this special, then?
There was a problem hiding this comment.
This is under the "Version Helpers" section, and is describing things which reference the old version stuff that would need adjusting for the new version stuff.
| | submodule | | | ||
| +-------------------------------------+-------------------------------------+ | ||
|
|
||
| We propose that a new package called extension-helpers be created starting from |
There was a problem hiding this comment.
I don't quite understand how this would work as @pllim is proposing. Then wouldn't we have to push updates to all users of this stuff if e.g. we need to update the OpenMP stuff?
| Impact for users | ||
| ---------------- | ||
|
|
||
| If done properly, these changes should have no noticeable impact for users. |
There was a problem hiding this comment.
As discussed in the coordination meeting, the one thing it will do is make it so that releases no longer contain c files. I'm OK with that, but it should be noted here so we're sure we all understand.
There was a problem hiding this comment.
I don't think this is the right section for that. I wouldn't consider someone who cares if the tarball contains pre-generated or auto-generated C files our regular user. I am also pretty sure we mention it further down the document.
There was a problem hiding this comment.
This is elaborated upon in this section.
APE17.rst
Outdated
| Alternatives | ||
| ------------ | ||
|
|
||
| In the words of Stuart: *Do nothing, suffer submodules and maintaining the |
There was a problem hiding this comment.
I think it's worth stating that pip will probably eventually break the helpers (if that's true?). That's a stronger "status quo won't work"/there are no alternatives argument.
| will be encouraged to have `tox`_ installed if they want to easily run | ||
| tests or build documentation locally. However, this is an easy package to | ||
| install with `pip`_ and we could also add code in ``setup.py`` so that running | ||
| ``python setup.py test`` or ``python setup.py build_docs`` gives a helpful |
There was a problem hiding this comment.
Is it worth pointing out that we can potentially keep these commands, if they translate into tox or pytest commands?
There was a problem hiding this comment.
I added a note to this effect in the "alternatives" section.
astrofrog
force-pushed
the
ape-astropy-helpers
branch
from
8307dfc to
4d84807
Compare
|
Thanks for addressing all my last bits there, @Cadair. And thanks to @astrofrog and @Cadair for all their hard work on this. Or as @Cadair would say: burn it all |
|
This APE has been accepted following unanimous approval at the 2019 coordination meeting. |
This is an APE describing a roadmap for transitioning away from astropy-helpers in favor of more standardize tools in the Python ecosystem. The short version is that:
Pure-Python packages that use astropy-helpers currently can already be refactored to not rely on astropy-helpers, and in fact several packages have already done so (see links in the implementation section).
Packages with C extensions can start to make some of the changes here but should wait a little before making the more significant changes related in particular to build-time dependencies since they require a recent version of pip (19.0 or later) - but we wanted to start the discussion now to start planning the changes.
A fraction of astropy-helpers would continue to exist as a more general extension-helpers package specifically geared towards packages with compiled extensions, and this could be used outside the astropy project, and would no longer be included as a sub-module.
Even though we can't make some of these changes in the core package yet, we wanted to start the discussion now so as to be able to make sure there are no issues in the current Python packaging ecosystem that would prevent these changes.
This is just a draft at this point, and we welcome detailed comments! If some points prove to be controversial, we'll try and set up a call so that we can discuss things and see if we can reach a consensus! Please also say if any aspects of this is confusing - some of us have spent far too much time thinking about this, and there may be some points that could be explained better.