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

Use --universal flag with uv pip compile #3032

Merged
merged 4 commits into from
Jul 20, 2024

Conversation

CoolCat467
Copy link
Member

Follow up to #2958

We might want to try out this new --universal flag (I'm not sure when it was added? but seems useful)

... but that can be a followup. This is good as a drop in replacement.

Originally posted by @A5rocks in #2958 (review)

This pull request is adding the new --universal flag for the uv pip compile runs.

Copy link

codecov bot commented Jul 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.63%. Comparing base (0c88bc7) to head (cd7af07).
Report is 115 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3032   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files         120      120           
  Lines       17830    17830           
  Branches     3204     3204           
=======================================
  Hits        17765    17765           
  Misses         46       46           
  Partials       19       19           

Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

The changes look good, I can't speak of whether they're accurate/missing things, but definitely something nice. I'm fine merging this despite there not being many docs about it, but once this is documented more we can probably close the issue RE: replacing pip-compile.

charset-normalizer==3.3.2
# via requests
click==8.1.7
# via towncrier
colorama==0.4.6 ; platform_system == 'Windows' or sys_platform == 'win32'
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

I'm not sure this is stable enough to use as is. I ran uv pip compile --universal --no-strip-markers --python-version=3.8 --upgrade test-requirements.in -o test-requirements.txt twice locally and it started duplicating requirements in the requirements file.

See astral-sh/uv#4885

@A5rocks
Copy link
Contributor

A5rocks commented Jul 8, 2024

Ok so while investigating that I figured out that I can't fix it (the fix would require too much change in uv I think).

However! 1) the reason we have different results is because this wasn't a problem prior to 0.2.19, and 2) a workaround (because we want to stay current) is to just put Sphinx at the top of the requirements file. Though I haven't double checked for 2.

@CoolCat467 CoolCat467 requested a review from A5rocks July 12, 2024 08:25
check.sh Outdated Show resolved Hide resolved
@CoolCat467
Copy link
Member Author

While looking through the documentation, I came across https://github.com/astral-sh/uv-pre-commit, might be interesting to try out using that.

@jakkdl
Copy link
Member

jakkdl commented Jul 17, 2024

While looking through the documentation, I came across https://github.com/astral-sh/uv-pre-commit, might be interesting to try out using that.

I think we want to bump dependencies in pull requests separate from other PRs, so adding a pre-commit check that plays around with requirements sounds like a bad idea to me.

charliermarsh pushed a commit to astral-sh/uv that referenced this pull request Jul 17, 2024
@CoolCat467 CoolCat467 requested a review from A5rocks July 18, 2024 04:24
@CoolCat467 CoolCat467 merged commit a3e8665 into python-trio:main Jul 20, 2024
35 checks passed
@CoolCat467 CoolCat467 deleted the uv-universal branch July 20, 2024 04:18
@CoolCat467
Copy link
Member Author

I think there is a case of test flakiness again, the merge run failed on pypy-3.10 with a dtls issue again:

   ______________________ test_gc_before_system_task_starts _______________________
  
      @pytest.mark.filterwarnings("always:unclosed DTLS:ResourceWarning")
      async def test_gc_before_system_task_starts() -> None:
          e = endpoint()
      
  >       with pytest.warns(ResourceWarning):
  E       Failed: DID NOT WARN. No warnings of type (<class 'ResourceWarning'>,) were emitted.
  E        Emitted warnings: [].
  
  /opt/hostedtoolcache/PyPy/3.10.14/x64/lib/pypy3.10/site-packages/trio/_tests/test_dtls.py:757: Failed

I think something similar to this happened recently elsewhere as well, might need to make an issue collecting test flakiness for pypy-3.10

@A5rocks
Copy link
Contributor

A5rocks commented Jul 21, 2024

Maybe we just have to bump GC counts yet again. (assuming that calls gc_harder)

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