Skip to content
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

PIG 13 - Gammapy dependencies and distribution #2218

Merged
merged 23 commits into from Sep 9, 2019

Conversation

@cdeil
Copy link
Member

commented Jun 6, 2019

This PIG is to make a clear plan and gather feedback concerning Gammapy dependencies and distribution.

Please give me a day to work on this, don't comment yet.

@cdeil cdeil added the pig label Jun 6, 2019
@cdeil cdeil added this to the 0.13 milestone Jun 6, 2019
@cdeil cdeil self-assigned this Jun 6, 2019
Copy link
Member

left a comment

some minor comments, nothing major

docs/development/pigs/pig-011.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-011.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-011.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-011.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-011.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-011.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-011.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-011.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-011.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-011.rst Outdated Show resolved Hide resolved
@cdeil

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2019

@bsipocz - thanks for the review. This is super early, the structure and text of the PIG isn't good yet. I'll ping you here again for review once it's in a shape where it makes sense.

There's one key question though that we should discuss this early already: @adonath and @registerrier - please have a look here and comment concerning which regions version we support. If we put region.rotate in now (see astropy/regions#265), it will be in regions v0.5 from June or July 2019 only. And if we want to clean up the rotated regions code in Gammapy before Gammapy v1.0, and not spend much time e.g. to bundle or monkey-path parts or regions or such horrible things, we'll depend on a super-recent version of regions. We'll have the same for astropy-healpix, where 1-2 small things are missing so that we can use it.

If we have these two Astropy-affiliated packages as dependencies, with versions from summer 2019, then the motivation to support very old version of Numpy, Scipy, Astropy is much reduced. The simplest option for us would be to require fairly modern versions there then, that are e.g. form 2017 or 2018. I'm in favour of this, because most of our users use conda, and there we just have a nice way to ship recent software on all platforms.

On the other hand, I don't think there really are things in the latest Numpy, Scipy, Astropy that we would take advantage of in Gammapy at this point - requiring modern versions there would mostly mean less effort on testing for old versions.

The other major system where Gammapy and the dependencies are packaged is Debian.
@olebole - could you please also have a look here and let us know what you recommend so that packaging Gammapy remains possible in Debian, i.e. we're not too agressive with new versions?

Our main goal is to have a good Gammapy v1.0 in Oct 2019 (see our roadmap), but I'd like to set up the dependencies and testing already now in the coming months to match what we plan to do for v1.0.

Thoughts?

@bsipocz

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

If we have these two Astropy-affiliated packages as dependencies, with versions from summer 2019, then the motivation to support very old version of Numpy, Scipy, Astropy is much reduced. The simplest option for us would be to require fairly modern versions there then, that are e.g. form 2017 or 2018. I'm in favour of this, because most of our users use conda, and there we just have a nice way to ship recent software on all platforms.

it can be pointed out in the docs of not supporting doesn't necessarily mean that old versions are broken with gammapy, rather that that they are not tested and regressions affecting only those old releases will most probably won't be fixed.

@bsipocz

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

(that would help to reduce the CI matrix significantly, currently travis totalling to ~3 hours is way to long)

@olebole

This comment has been minimized.

Copy link

commented Jun 7, 2019

The other major system where Gammapy and the dependencies are packaged is Debian.
@olebole - could you please also have a look here and let us know what you recommend so that packaging Gammapy remains possible in Debian, i.e. we're not too agressive with new versions?

In principle, we try to keep up-to-date with our testing/unstable releases (with the exception of the current "freeze", where we don't update versions). The versions in the link look therefore completely OK.

Two general (minor) points here, however: First, a bug that appeared in Ubuntu (astropy/astropy#8783) made me think about being a bit more conservative (with no solution yet; input is welcome).

Second, apart from Ubuntu (which comes out 6monthly), the next (post-Buster) Debian release will be out in more than two years only, and Debian Buster will carry gammapy 0.10 until then. In principle, it can be "backported" (made available in a special Backports add-on repository), but this requires to have the dependencies available, so they may need to be backported as well. Hard dependencies on the newest, shiniest versions everywhere makes this harder (up to impossible when there are conflicts f.e. in the Python version). So, I can only give the general recommendation to keep the dependency versions a bit relaxed if possible, especially when they require big updates in turn. However, handling backports is still not systematic anyway (volunteers ... where one can find volunteers...?)

@cdeil cdeil changed the title PIG 11 - Gammapy dependencies and distribution PIG 13 - Gammapy dependencies and distribution Jun 17, 2019
@cdeil

This comment has been minimized.

Copy link
Member Author

commented Jun 17, 2019

@olebole and @bsipocz - Thanks for the feedback!

Concerning Debian and Linux distributions: my feeling is that it's not worth to spend time on backports. We don't have anyone familiar with Linux packaging in the Gammapy team, can't contribute there at the moment. And Gammapy will change a ton in the next half year until v1.0 at least, with releases every 2 months. We don't have the time to support old releases, we're trying to go as fast as we can towards a good v1.0 in October 2019. That's just not appropriate for Linux distributions which have ~ yearly update cycles. After v1.0 and in the coming years the story might be very different, as Gammapy becomes stable having Debian packages becomes very valuable.

One thing that changed just now is that Tom set up https://github.com/astropy/wheel-forge so that binary wheels for all Gammapy dependencies can be distributed. If we can get in on that service, I think we should try it for a bit and distribute binary wheels as well. That's a second means besides conda to distribute quickly on all platforms, and it should work well on top of e.g. a system Python on Debian for now, while our code changes too rapidly for the system packaging there.

I will update this PIG in the coming days to a useful draft state, then ask for review again.

One more thing: I noticed that PIG 11 was already given the the lightcurve PIG in #1451 .
So I'll change this PIG on Gammapy dependencies and distribution now to be PIG 13.

@cdeil cdeil requested review from adonath and registerrier Jun 17, 2019
Copy link
Member

left a comment

Thanks @cdeil! I've left a few minor inline comments, my main comment would be the to make the definition of "required" and "optional" dependencies clearer (apart from the difference in install and import). May also better motivate the choice of "pyyaml" and "click" dependencies, as there are alternatives.

docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Show resolved Hide resolved
docs/development/pigs/pig-013.rst Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
@cdeil

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2019

@adonath @bsipocz - I think I've addressed all of your comments in 30cdf2c .

Please give me a day or two to finish the PIG before reading and commenting again. I'll post a comment here when it's ready for review by the co-authors, hopefully already tomorrow.

@cdeil

This comment has been minimized.

Copy link
Member Author

commented Jul 3, 2019

@Bultako @jasercion @bsipocz @astrofrog or @mackaiver (xref #1245) or anyone - what do you think is the best way to ship both a core package with required dependencies, as well as an analysis stack that includes optional dependencies, with both pip and conda?

If you have time to read this draft proposal section for Gammapy and leave a comment here, that would be super helpful!

@cdeil

This comment has been minimized.

Copy link
Member Author

commented Jul 4, 2019

Advice from Tom Robitaille via Astropy Slack:

For pip, I prefer extras_require if it's for actual optional dependencies to a single package, and a metapackage on PyPI if you want to install multiple different gammapy components. For conda, I think a meta-package is probably the best way if you don't want to install all the optional dependencies with the default build. In my experience, requirements.txt (for pip) and environment files (for conda) will only be usable by a small fraction of users.

Just to expand on the metapackage vs extras_require difference for pip, I'd say that if it's that e.g. MPL or scipy are optional dependencies, that should be an extras_requires. But for e.g. glue, I have a glueviz meta-package that installs the core glue package as well as any plugins I want to recommend to users.

@kbruegge

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2019

Just pinging @MaxNoe. I'm not an expert in python packaging.

@MaxNoe

This comment has been minimized.

Copy link

commented Jul 4, 2019

So from my understanding, packages should not use requirements.txt, this is something for applications or just some random scripts that have no setup.py.

For packages, dependencies should be put in the install_requires, setup_requires etc. arguments of the setup function.

@cdeil

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

@adonath @registerrier @bsipocz as co-authors, or anyone that has time now: Please review the first complete draft of the PIG here.

If possible, I would like to send this for review to the Gammapy team and users soon, but of course before that we should agree on the proposal, so please focus comments on the main questions.

One change I made, based on the feedback from Tom Robitaille and @MaxNoe above is to not propose to create and distribute requirements.txt files to users via gammapy.org, but instead to define an "all" target and have pip install gammapy[all]. I've also made it clear that this will be an experimental way to distribute Gammapy, it's not clear if we have time to work on Wheels for now, or how well those will work. So I mention it as something we want to work on, but it's not a requirement or blocker for Gammapy v1.0.

For conda there's the same question. For now I'm suggesting we stick to what we have with the env file, because it's been working well for us and as far as I know users like it. However, I do mention the option to change to or add a conda meta-package called gammapy-all as something we invite comments on. If someone is interested to create that and maintain it, we could also just try it out. To me it's not clear how maintenance / versioning would work exactly, e.g. in the current env files we sometimes found little issues post release and just updated the dependency versions there via "stealth updates", i.e. without a patch version number like "0.12.1"; is this possible with conda meta-packages our would we have to tag bugfix versions there?

The core of the proposal remains for dependencies: we clearly list which ones we have, and propose adopting recent versions, which will help us clean up the code and focus more on features than on maintenance in the coming months.

Copy link
Contributor

left a comment

Thanks @cdeil . This looks good to me.

Some minor comments inline.

docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
docs/development/pigs/pig-013.rst Outdated Show resolved Hide resolved
@MaxNoe MaxNoe referenced this pull request Jul 9, 2019
@cdeil

This comment has been minimized.

Copy link
Member Author

commented Sep 9, 2019

Comment period for this PIG ended yesterday: https://groups.google.com/forum/#!topic/gammapy/B7Cj761enN4
We didn't get any further comments.
I'll merge this in now, open reminder issues for the tasks for myself and implement them within the next week.

@cdeil cdeil force-pushed the cdeil:pig-11 branch from 1536061 to 18ec335 Sep 9, 2019
@cdeil cdeil merged commit 385733e into gammapy:master Sep 9, 2019
6 of 9 checks passed
6 of 9 checks passed
Codacy/PR Quality Review Hang in there, Codacy is reviewing your Pull request.
Details
Scrutinizer Created
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
gammapy.gammapy Build #20190909.7 succeeded
Details
gammapy.gammapy (DevDocs) DevDocs succeeded
Details
gammapy.gammapy (Lint) Lint succeeded
Details
gammapy.gammapy (Test Python36) Test Python36 succeeded
Details
gammapy.gammapy (Test Windows36) Test Windows36 succeeded
Details
gammapy.gammapy (Test Windows37) Test Windows37 succeeded
Details
@adonath

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

🎉

@bsipocz

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

@cdeil - this is now water under the bridge, but skimming through the PIG again, I started to wonder what will happen in the future? E.g. this PIG doesn't set up rules for future dependency version drops. Is the plan to set up a table for each new (major) version, or rather define a set of rules?

@cdeil

This comment has been minimized.

Copy link
Member Author

commented Sep 9, 2019

The idea is to transfer the tables to the docs, and then to maintain them there (likely making incremental changes in pull requests without much discussion).

We don't have a policy or something like that for the coming years yet, that will come later.

This proposal was just to have an opportunity for discussion and feedback and be transparent about this one-time change.

@bsipocz

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

OK, that sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.