New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
How to handle AstroPy changes breaking affiliated package dependencies? #369
Comments
FWIW, here's a long argument against what I suggested above: Maybe it's not the best approach, but it would be good to have a policy on that 🙂. |
Correct me if I'm wrong but I think this is a general issue with Python
builds and dependency resolution. I believe there's no general way to solve
the issue and as you mentioned there are good reasons not to constrain
package versions.
The approach we've adopted is
- Constrain application dependencies on the right
- Do not constrain dependencies in libraries
So jwst may constrain
gwcs>=0.19.0,<0.20.0
but normally gwcs will not constrain astropy and it is assumed that
applications will do it if necessary.
This seems to work but of course one has to remember to update dependencies.
I'm curious to hear how others deal with the issue.
|
Indeed, though this surprised me a bit, given that astropy & gwcs are closely related packages (maybe an unrealistic expectation). Ah, that's interesting about applications -- but if that's the assumption then I suppose other projects that use the libs need to know about it. Here DRAGONS 3.1 would have had to constrain |
Thanks for opening this issue and sharing your thoughts! I agree with Nadia in the sense that no amount of pinning would be foolproof. Some sort of integration testing would have helped but the combo that works for you might be meaningless for a different pipeline. If you want to be extra safe, you can do exact ( @nden , didn't your team deploy https://github.com/spacetelescope/stenv ? Does that include pinning? How is that working out for you? tl;dr -- My personal opinion is that this should be handled downstream, not in |
One additional comment here: Astronomy packages often face the danger to be abandoned by their authors, or at least seem orphaned for quite a while. Very often, they can however still survive because they use only a subset API of their dependencies. Having upper version limits for these dependencies would make them unusable long before they really get buggy with the new version. I just had the exercise to check the reverse dependencies of Astropy 6.0 in Debian if they still work for the released packages. Result (the list above only contains the failures):
Most of the failures were caused by test code that needed an update (exception: pyvo). So, I think that having general upper constraints on dependencies is more harmful than it helps. Downstream, one could do that if a DeprecationWarning appears and one is not able to fix that. What is always helpful: include the tests in the package itself. So, if a user has doubts that a package still works after update, they can always run a |
There was never a reason to pin
|
Hmm, that should have been properly deprecated, or was there a mistake made somewhere? Anyway, new pyvo release should fix stuff, we probably get around it next week or the one after that |
In 5.3.4, votable.tree.Table was not marked as deprecated in the source, and also not in CHANGES.rst. |
It should not be landed in 5.3.4, but 6.0 and astropy/astropy#15372 should have dealt with doing the deprecation rather than renaming it only. Was any of it refactored further before the release? |
Re: pyvo Looking at the log Ole posted on https://github.com/astropy/astropy/wiki/v6.0-RC-Testing#testing-with-coordinatedaffiliatedother-downstream-packages-1
I think it is just a matter of outdated test because this patch was not in the pyvo version under test: - assert isinstance(dalresults.resultstable, VOTable)
+ assert isinstance(dalresults.resultstable, TableElement) In this scenario, pyvo is expecting the deprecated VOTable (which is the Therefore, a new pyvo release (where the test is already updated) would solve the pyvo test failure for Ole. |
That is correct. Because it was not deprecated until astropy 6.0.0. You would see that change log for 6.0.0 but not in 5.3.4. |
I can't check on this now, but as I recall this line is misleading as Anyway, this should not be an issue for any users whatsoever. |
I agree on this front. Just a matter of the test needing an update. So, Ole, I think you can even ignore that one test for pyvo for now and your distro would theoretically be fine. |
I'll probably just apply the patch VOTable-->TableElement in the test, thank you for the hint. And sorry that this hijacked the discussion topic here. To bring it back on path: IMO this example supports my argument that forward compatibility is often not a real issue, and causes less problems than blindly applying an upper limit on a dependency. |
No problem; it's quite a general discussion. Thanks for everyone's input. I'm just wondering whether we should write down these expectations somewhere (including for users). Eg. I see there's a whole new installation section here, where it might be worth mentioning that dependencies are not guaranteed to work if you update astropy as described without updating them too?: |
When testing astropy 6, I noticed that it's possible to get a (partly) broken installation by updating astropy while an older, incompatible gwcs version <0.18 is still installed.
It seems that gwcs versions <0.18 don't work with astropy 6, because they depend on functionality that was later split into the stand-alone
asdf-astropy
package, which they don't know about. This causes asdf serialization of gwcs objects to fail. While it's the responsibility of gwcs to specify what astropy versions are compatible, it obviously cannot know about all future changes upstream, nor can the dependency constraints of already-published gwcs versions be modified reliably after the fact (because the dependency solver can always choose an older build, before the extra constraint was added, unless old builds are removed from the package repo, which is a Bad Idea for other reasons). Thus, since astropy does not depend on gwcs in turn, there is nothing to prevent the broken combination.When building the astropy conda package, this particular issue could be avoided by adding
"run_constrained: gwcs >=0.18"
to the recipe, which would disallow installation alongside incompatible gwcs versions, without actually making gwcs into a dependency. However, I'm not sure whether a similar mechanism exists for pip -- and it's too late now anyway, because 6.0 was already published on PyPI yesterday. So this would only be a partial solution.This is not the end of the World, because users doing a typical new install should get a working combination of packages, but it would be better if the breakage could not occur when updating
astropy
or including additional packages that could hold gwcs back.I'm therefore wondering whether affiliated/dependent packages should perhaps be using constraints such as
astropy >=5.0,<6.0
(as conda does for python & numpy), rather than leaving the maximum version wide open, in anticipation of such future breakage? Or maybe some of you have other insights regarding best practices for avoiding dependency hell? I might be making a meal out of a very general problem without a general solution, but it's at least good to be aware of the issue, in order to tell users what to update when they run into it.(I suppose for DRAGONS, we'll just require
gwcs >=0.18
when we publish v3.2 next year and then the issue will mostly go away for us.)cc: @saimn, @pllim, @mwcraig, @astrofrog, @nden, @olebole
The text was updated successfully, but these errors were encountered: