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

Pin pyproject dependencies #2154

Merged

Conversation

jemrobinson
Copy link
Member

@jemrobinson jemrobinson commented Aug 22, 2024

✅ Checklist

  • You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
  • You are targeting the appropriate branch. If you're not certain which one this is, it should be develop.
  • Your branch is up-to-date with the target branch (it probably was when you started, but it may have changed since then).

🚦 Depends on

n/a

⤴️ Summary

  • Pin Python package dependencies in pyproject.toml
  • Drop custom pre-installation of pinned versions
  • Drop custom updating scripts in favour of dependabot
  • Use recommended structure for environment requirements: group them into optional feature groups which means they can (if desired) be installed with e.g. pip install ".[test]"

Justification:

  • "Should I pin my dependencies?" discussion here and here
    • summary: yes, if you're writing an application that isn't expecting other packages to depend on it

Potential issues:

  • This only pins direct dependencies, not the things that those packages depend on
    • I think this is OK, as this shouldn't affect the functions we're calling. If it does, then we probably have a (hidden) direct dependency and should probably add that to pyproject.toml
    • Solved by using hatch-pip-compile which is a built-in version of what we were previously doing manually
  • Dependabot might try to update the pinned requirements files
    • Not sure whether this is a problem or not

🌂 Related issues

Part of #2084

🔬 Tests

Tested on a fork of this repository - dependabot is updating pyproject.toml as expected.

@jemrobinson jemrobinson requested review from a team as code owners August 22, 2024 12:07
Copy link

Coverage report

This PR does not seem to contain any modification to coverable code.

@jemrobinson jemrobinson force-pushed the 2084-make-pip-installable branch 8 times, most recently from 5871fe9 to e4fa6be Compare August 22, 2024 13:21
Copy link
Member

@JimMadge JimMadge left a comment

Choose a reason for hiding this comment

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

I don't think this is the way I want to go.

We loose the pinned dependencies here as we only have direct dependencies. That would make debugging more difficult and docs builds unreproducible.

I think there is an advantage to having the minimal dependencies for development (for example, when we had to set a minimum patch version for pulumi-azure-native) and the full pinned dependencies. pip-tools feels like a good, hosting agnostic way to do that.

A better solution might be to figure out how to support a requirements.in with the full dependencies going to pyproject.toml rather than requirement.txt

@jemrobinson
Copy link
Member Author

jemrobinson commented Aug 22, 2024

In principle pinning direct dependencies should be OK. If any of the functions from our directly-called libraries change their behaviour because of the specific version of their dependencies then we actually depend on that library too and we should be explicit about that.

I disagree that having minimal dependencies listed in pyproject.toml is helpful. We actually have no idea whether the set of constraints in this file were correct, because we never used them in development - we were always installing from requirements.txt.

@jemrobinson
Copy link
Member Author

@JimMadge : Transitive dependencies now pinned using hatch-pip-compile

@jemrobinson jemrobinson force-pushed the 2084-make-pip-installable branch 2 times, most recently from e45b32c to 57d002a Compare August 22, 2024 15:01
@jemrobinson jemrobinson force-pushed the 2084-make-pip-installable branch from dfb3480 to e58dd19 Compare August 22, 2024 15:18
@jemrobinson jemrobinson requested review from craddm, a team and JimMadge August 22, 2024 15:23
pyproject.toml Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
Copy link
Member

@JimMadge JimMadge left a comment

Choose a reason for hiding this comment

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

Happy to merge this as it doesn't look like there is a sensible way to do what we would ideally want.

We can review that again in the future.

@jemrobinson jemrobinson merged commit d6358f9 into alan-turing-institute:develop Aug 29, 2024
11 checks passed
@jemrobinson jemrobinson deleted the 2084-make-pip-installable branch January 30, 2025 11:46
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.

2 participants