Skip to content

Conversation

@bdraco
Copy link
Member

@bdraco bdraco commented Sep 2, 2024

What do these changes do?

Add typing to URL

Are there changes in behavior for the user?

no runtime changes


def with_query(self, *args, **kwargs):
@overload
def with_query(self, query: _Query) -> "URL": ...

Check notice

Code scanning / CodeQL

Statement has no effect

This statement has no effect.
def with_query(self, query: _Query) -> "URL": ...

@overload
def with_query(self, **kwargs: _QueryVariable) -> "URL": ...

Check notice

Code scanning / CodeQL

Statement has no effect

This statement has no effect.

def update_query(self, *args, **kwargs):
@overload
def update_query(self, query: _Query) -> "URL": ...

Check notice

Code scanning / CodeQL

Statement has no effect

This statement has no effect.
def update_query(self, query: _Query) -> "URL": ...

@overload
def update_query(self, **kwargs: _QueryVariable) -> "URL": ...

Check notice

Code scanning / CodeQL

Statement has no effect

This statement has no effect.
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Sep 2, 2024
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
@codecov
Copy link

codecov bot commented Sep 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.88%. Comparing base (d4e7601) to head (24768b8).
Report is 318 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1084      +/-   ##
==========================================
+ Coverage   93.06%   94.88%   +1.82%     
==========================================
  Files          31       30       -1     
  Lines        4424     4358      -66     
  Branches      370      382      +12     
==========================================
+ Hits         4117     4135      +18     
+ Misses        281      197      -84     
  Partials       26       26              
Flag Coverage Δ
CI-GHA 94.83% <100.00%> (+1.82%) ⬆️
MyPy 38.91% <97.50%> (+11.94%) ⬆️
OS-Linux 99.34% <99.12%> (+<0.01%) ⬆️
OS-Windows 99.44% <99.12%> (+<0.01%) ⬆️
OS-macOS 99.02% <99.12%> (+<0.01%) ⬆️
Py-3.10.11 98.90% <98.24%> (-0.03%) ⬇️
Py-3.10.14 99.16% <98.24%> (-0.03%) ⬇️
Py-3.11.9 99.16% <98.24%> (-0.03%) ⬇️
Py-3.12.5 99.16% <98.24%> (-0.03%) ⬇️
Py-3.13.0-rc.1 99.16% <98.24%> (-0.03%) ⬇️
Py-3.8.10 98.85% <98.24%> (-0.03%) ⬇️
Py-3.8.18 99.11% <98.24%> (-0.03%) ⬇️
Py-3.9.13 98.85% <98.24%> (-0.03%) ⬇️
Py-3.9.19 99.11% <98.24%> (-0.03%) ⬇️
Py-pypy7.3.11 99.16% <98.24%> (-0.03%) ⬇️
Py-pypy7.3.16 99.19% <98.24%> (-0.03%) ⬇️
VM-macos-latest 99.02% <99.12%> (+<0.01%) ⬆️
VM-ubuntu-latest 99.34% <99.12%> (+<0.01%) ⬆️
VM-windows-latest 99.44% <99.12%> (+<0.01%) ⬆️
pytest 99.34% <99.12%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

webknjaz and others added 4 commits September 3, 2024 02:48
@bdraco
Copy link
Member Author

bdraco commented Sep 3, 2024

@bdraco bdraco marked this pull request as ready for review September 3, 2024 05:54
@webknjaz
Copy link
Member

webknjaz commented Sep 3, 2024

path shows 100% diff hit on codecov but is reporting at 99.13% here...

Yeah, I suppose this was due to the proportion being different because the overall number of lines changed. I'm not seeing 99.13% there, though. Perhaps, it processed more reports after your comment.
For me, it shows 94.94% for the entire pytest flag and 99.88% for tests/ flagged with pytest and 98.15% for yarl/+pytest.

I suppose, it's okay to raise the codecov/project/lib expectation to 98.1% (leaving that 0.05% margin for future line count changes, until we can go for 100%).

@bdraco bdraco merged commit e70f9d4 into master Sep 3, 2024
@bdraco bdraco deleted the typing branch September 3, 2024 19:40
@bdraco
Copy link
Member Author

bdraco commented Sep 3, 2024

I'm going to do a RC0 release so I can run this in the CI of a few projects since typing changes are always hard to predict the impact

@sergeniushpe
Copy link

sergeniushpe commented Sep 5, 2024

Hello, I think this change broke python 3.8 support:

../lib/python3.8/site-packages/yarl/_url.py:606: in URL
    def query(self) -> MultiDictProxy[str]:
E   TypeError: 'type' object is not subscriptable

If I downgrade to release 1.9.7 it works again.

You might want to update python requirements to indicate python>=3.9 is required

@bdraco
Copy link
Member Author

bdraco commented Sep 5, 2024

Hello, I think this change broke python 3.8 support:

../lib/python3.8/site-packages/yarl/_url.py:606: in URL
    def query(self) -> MultiDictProxy[str]:
E   TypeError: 'type' object is not subscriptable

If I downgrade to release 1.9.7 it works again.

You might want to update python requirements to indicate python>=3.9 is required

Would you please open an issue report, I will fix it shortly

@bdraco
Copy link
Member Author

bdraco commented Sep 5, 2024

Please include the multi dict version you are using in the issue

@Dreamsorcerer
Copy link
Member

Please include the multi dict version you are using in the issue

As far as I can see, multidict has supported that syntax for ~6 years...
But, maybe we should increase the minimum version of multidict that yarl supports.

@bdraco
Copy link
Member Author

bdraco commented Sep 5, 2024

I had to take the python 3.8 install back to multidict 4 to get it to fail. There might be a newer version that fails though as I stopped trying them once I confirmed the issue. Also even with 4 it didn’t fail on python 3.9 so I suspect the issue will go away on its own soon when we drop 3.8

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

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants