Skip to content

DEP: add missing lower bounds to direct dependencies - #100

Closed
neutrinoceros wants to merge 5 commits into
astropy:mainfrom
neutrinoceros:dep/lower-bounds
Closed

DEP: add missing lower bounds to direct dependencies#100
neutrinoceros wants to merge 5 commits into
astropy:mainfrom
neutrinoceros:dep/lower-bounds

Conversation

@neutrinoceros

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread CHANGES.md
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Section format should be

## X.Y (unreleased)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should I use a literal X.Y or am I supposed to guess what the next version number should be ?

Comment thread CHANGES.md
## Unreleased

- Python 3.10 or newer is now required [#94]
- Lower bounds were added to all direct dependencies [#100]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How were this bounds decided?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran uv pip install . --resolution=lowest and iteratively added packages to --only-binary='<pkgs>' until the only thing that didn't install from a wheel was sphinx-astropy itself. Then I bumped 3 direct dependencies some more (each bump is its own commit) to their oldest versions that didn't require setuptools at runtime (because I'm trying to eliminate it from astropy's own runtime tree)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sphinx-astropy is also used downstream that might need older versions. Might need to ask the astropy-dev list about this PR, just in case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if anyone needs older versions why would they need to upgrade sphinx-astropy ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot guess people's motivation. All I am saying is that we don't usually bump minversions in auxiliary packages like this one unless we really have to, and we need to think of packages that are not astropy.

@neutrinoceros neutrinoceros May 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course. But it's also impossible to tell what users are currently expecting precisely because there are no lower bounds in the first place. I would expect any breakage caused by such a change to be very unlikely, and if it happened anyway, the likelyhood that we can avoid it by trying to reach out first seems vanishingly small. I honestly don't think it's worth the trouble, when the alternative (revert a bump and re-release) is extremely cheap.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please dont' mess with these aux packages; we do pin to lower bounds if/when they are needed for a reason. These are available as a collection of other packages, there is absolutely no need for overengineering and adding version limits here.

@neutrinoceros
neutrinoceros deleted the dep/lower-bounds branch May 31, 2026 11:58
@neutrinoceros

Copy link
Copy Markdown
Contributor Author

I don't think the resistance met here is justified but I'm also not going to fight over it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants