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

Update dependencies versions #17

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

HealthyPear
Copy link
Member

I recently had to re-install this package in a new environment and encountered some issues.

The very old pinned version of astropy was triggering a crash from setuptools which only triggered a cascade of more issues when trying to fix it.

All dependencies' versions were pinned and this leaves very small range to development: I propose here to pin only the core ones (astropy and numpy and leave thew rest to pip - or conda).
I also propose to bump the minimum version of Python from 3.8 to 3.9 as the former will reach end-of-life in about 4 months.

Also fixes #8.

Testing (see #16) should uncover any issue with these changes and only then it will make sense to pin versions.

Pin only core dependencies and leave Python 3.8
@HealthyPear HealthyPear changed the title Update pyproject.toml Update dependencies versions Jun 27, 2024
@HealthyPear HealthyPear marked this pull request as draft June 27, 2024 09:45
@HealthyPear HealthyPear marked this pull request as ready for review June 27, 2024 09:46
@mstrzys
Copy link
Collaborator

mstrzys commented Jul 2, 2024

I very much agree with the concept that Unit testing shall find problematic changes in the external modules and to drop the pinned versions until such issue come up. Following this idea, should we really keep the constrains for astropy and numpy?

Dropping python 3.8 is fine, given that lstchain and gammapy require 3.11, 3.9 upwards should be more than tolerant enough. Since pybkgmodel is supposed to be used with these packages, most users will anyway use the version pinned by them.

@HealthyPear
Copy link
Member Author

I very much agree with the concept that Unit testing shall find problematic changes in the external modules and to drop the pinned versions until such issue come up. Following this idea, should we really keep the constrains for astropy and numpy?

I thought that maybe at least pinning astropy might be a good idea, since there are usually breaking changes between major releases (when I say "pin" I also mean simply what I did here, e.g. making sure astropy is recent enough, but preventing an automatic upgrade to the next major release, not really pinning a specific version).
I know numpy 2.0 caused a bit of issues in some other open-source projects, so I thought it might be beneficial to do the same for that dependency.

@HealthyPear
Copy link
Member Author

HealthyPear commented Jul 18, 2024

What is the policy of this project regarding merging pull requests? is one approval sufficient? should we fix the threshold to 2?

@IevgenVovk
Copy link
Collaborator

This wasn't agreed upon. A minimal sufficient practice would be if the PR author request review from developers of his / her choice (those can add others if required) and PR is merged following their approval. We can indeed require at least to developers to concur, though.

@HealthyPear
Copy link
Member Author

I think that at least until we have a solid test/benchmarks suite 2 approvals should be required just to make sure...

@HealthyPear HealthyPear added this to the 0.1.0 milestone Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installation fails with python 3.11.4
3 participants