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

Upgrade deps. #104

Merged
merged 1 commit into from
Dec 10, 2024
Merged

Upgrade deps. #104

merged 1 commit into from
Dec 10, 2024

Conversation

jsirois
Copy link
Contributor

@jsirois jsirois commented Dec 10, 2024

This ~dogfoods Pex 2.25's --elide-unused-requires-dist which prompted
the fix in Pex 2.25.1 here: pex-tool/pex#2615

@@ -191,6 +191,7 @@ def maybe_create_lock(session: Session) -> bool:
"latest",
"--resolver-version",
"pip-2020-resolver",
"--elide-unused-requires-dist",
Copy link
Contributor Author

@jsirois jsirois Dec 10, 2024

Choose a reason for hiding this comment

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

After the edits in this file, the rest of this change was generated via rm lock.json && nox -e lock (with attention paid to the comment in nox-support/doc-reqs.windows-amd64.lock.txt:

# N.B.: The 21c3f448d17224246e36a5db678e98b9261a1909604c160cd12f45fa1e7eb695 hash
# was added manually. This is the hash of the 1.1.2 wheel which does not match the
# VCS requirement this was locked from. The end result is docgen works on Windows
# until https://github.com/vsalvino/sphinx-library/pull/3 is released and we can
# switch back to the PyPI release.
sphinx-library==1.1.2 \
--hash=sha256:f6bb070fbc9a42482197dfdfa29c01e9e245822a86f17541f5a6dcee098cfe1a \
--hash=sha256:21c3f448d17224246e36a5db678e98b9261a1909604c160cd12f45fa1e7eb695

@jsirois jsirois requested review from kaos and kwlzn December 10, 2024 18:49
"pytest-cov; extra == \"dev\"",
"pytest>=6.0; extra == \"dev\"",
"pytz>=2015.7; python_version < \"3.9\""
"pytest>=6.0; extra == \"dev\""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to clarify my understanding of the pex feature:

freezegun and pytest-cov were filtered out because the dev extra of babel was not (transitively) requested via the input requirements? What I'm actually asking is, why is pytest is still here?

Copy link
Contributor Author

@jsirois jsirois Dec 10, 2024

Choose a reason for hiding this comment

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

The pytest dep is still there because its an input requirement:

lift/lock.json

Lines 1865 to 1883 in 9edff13

"requirements": [
"appdirs",
"autoflake",
"beautifulsoup4",
"black",
"click",
"click-log",
"click_didyoumean",
"docutils",
"filelock",
"httpx",
"isort",
"mypy",
"myst-parser[linkify]",
"nox",
"packaging",
"psutil",
"pytest",
"pytest-xdist",

That's a bug though since the dev extra is never activated. In other words, the feature currently doesn't elide enough and you can be left with 1/2 an extra set. If you then subsetted via pex --lock lock.json foo[dev] -o foo.pex you'd only get part of the dev extra, which is certainly wrong. All or nothing seem correct, and since the dev extra was not requested via the input requirements, it should be nothing, aka Pex should error since the dev extra was never locked (in full).

I'll follow up both in Pex and here after. I had a fancier elision scheme in 2.25.0 that was mostly correct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that's what I was grasping at. Even if pytest can be brought in by some other dep path, there is no need to mention it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, fixed up in #105.

@jsirois jsirois merged commit ad038e6 into a-scie:main Dec 10, 2024
7 checks passed
@jsirois jsirois deleted the lock/update branch December 10, 2024 20:16
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