Skip to content

Add new method without_query_params to drop query keys#898

Merged
bdraco merged 40 commits intoaio-libs:masterfrom
hongquan:feature/drop-query-keys
Sep 6, 2024
Merged

Add new method without_query_params to drop query keys#898
bdraco merged 40 commits intoaio-libs:masterfrom
hongquan:feature/drop-query-keys

Conversation

@hongquan
Copy link
Copy Markdown
Contributor

@hongquan hongquan commented Jun 23, 2023 •
edited by bdraco
Loading

What do these changes do?

Add a method, without_query_params to drop some keys from query part.
For example, dropping "b" in "a=1&b=2&c=3", we get "a=1&c=3".

Are there changes in behavior for the user?

No.

Related issue number

closes #774
closes #1010

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 16, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.05%. Comparing base (d080a2e) to head (91a8f62).
Report is 299 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #898      +/-   ##
==========================================
+ Coverage   95.02%   95.05%   +0.03%     
==========================================
  Files          30       30              
  Lines        4459     4490      +31     
  Branches      392      396       +4     
==========================================
+ Hits         4237     4268      +31     
  Misses        196      196              
  Partials       26       26
Flag Coverage Δ
CI-GHA 95.01% <100.00%> (+0.03%) ⬆️
MyPy 40.33% <100.00%> (+0.44%) ⬆️
OS-Linux 99.35% <100.00%> (+<0.01%) ⬆️
OS-Windows 99.45% <100.00%> (+<0.01%) ⬆️
OS-macOS 99.04% <100.00%> (+<0.01%) ⬆️
Py-3.10.11 98.93% <100.00%> (+<0.01%) ⬆️
Py-3.10.14 99.18% <100.00%> (+<0.01%) ⬆️
Py-3.11.9 99.18% <100.00%> (+<0.01%) ⬆️
Py-3.12.5 99.18% <100.00%> (+<0.01%) ⬆️
Py-3.13.0-rc.1 99.18% <100.00%> (+<0.01%) ⬆️
Py-3.8.10 98.87% <100.00%> (+<0.01%) ⬆️
Py-3.8.18 99.13% <100.00%> (+<0.01%) ⬆️
Py-3.9.13 98.87% <100.00%> (+<0.01%) ⬆️
Py-3.9.19 99.13% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.11 99.18% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.16 99.18% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.17 99.21% <100.00%> (+<0.01%) ⬆️
VM-macos-latest 99.04% <100.00%> (+<0.01%) ⬆️
VM-ubuntu-latest 99.35% <100.00%> (+<0.01%) ⬆️
VM-windows-latest 99.45% <100.00%> (+<0.01%) ⬆️
pytest 99.35% <100.00%> (+<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.

@mjpieters
Copy link
Copy Markdown
Contributor

mjpieters commented Jul 16, 2023

I really do appreciate that you put in the work for a PR here, but I'd like to first see more discussion about what the API should look like. Can you please first propose this API change in the issue?

(I'm not 100% convinced that the name drop_query_keys() is right here; perhaps without_query_keys() is better as you are returning a new URL rather than mutating the existing URL in-place).

@hongquan
Copy link
Copy Markdown
Contributor Author

hongquan commented Jul 16, 2023

Thanks. I think without_query_keys is a better name.

@mjpieters mjpieters requested a review from asvetlov July 16, 2023 18:41
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Aug 30, 2023
@webknjaz webknjaz requested a review from mjpieters November 19, 2023 22:59
@webknjaz webknjaz force-pushed the feature/drop-query-keys branch from 49c3b82 to 01091a6 Compare November 19, 2023 23:08
webknjaz
webknjaz reviewed Nov 19, 2023
View reviewed changes
Comment thread docs/api.rst Outdated
webknjaz
webknjaz reviewed Nov 19, 2023
View reviewed changes
Comment thread CHANGES/898.feature.rst Outdated
webknjaz
webknjaz reviewed Nov 19, 2023
View reviewed changes
Comment thread tests/test_url_query.py Outdated
webknjaz
webknjaz reviewed Nov 19, 2023
View reviewed changes
Comment thread yarl/_url.py
@webknjaz
Copy link
Copy Markdown
Member

webknjaz commented Nov 19, 2023

@mjpieters @hongquan how would you feel about URL.without_query_params()? The RFCs usually use this term...

@webknjaz webknjaz linked an issue Nov 20, 2023 that may be closed by this pull request
Ability to remove a query value while retaining others #774
Closed
1 task
@hongquan
Copy link
Copy Markdown
Contributor Author

hongquan commented Nov 21, 2023

Agree with without_query_params name.

webknjaz
webknjaz reviewed Dec 3, 2023
View reviewed changes
Comment thread CHANGES/898.feature.rst Outdated
webknjaz
webknjaz reviewed Dec 3, 2023
View reviewed changes
Comment thread tests/test_url_query.py
webknjaz
webknjaz reviewed Dec 3, 2023
View reviewed changes
Comment thread yarl/_url.py
webknjaz
webknjaz reviewed Dec 3, 2023
View reviewed changes
Comment thread yarl/_url.py Outdated
webknjaz
webknjaz reviewed Sep 6, 2024
View reviewed changes
Comment thread yarl/_url.py
webknjaz
webknjaz reviewed Sep 6, 2024
View reviewed changes
Comment thread yarl/_url.py
bdraco and others added 3 commits September 6, 2024 10:24
@bdraco @webknjaz
Update yarl/_url.py
fe2772e 
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
@bdraco @webknjaz
Update yarl/_url.py
86c34dc 
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
@bdraco @webknjaz
Update docs/api.rst
0bc4eef 
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
bdraco
bdraco reviewed Sep 6, 2024
View reviewed changes
Comment thread tests/test_url_query.py Outdated
bdraco
bdraco reviewed Sep 6, 2024
View reviewed changes
Comment thread tests/test_url_query.py Outdated
bdraco
bdraco reviewed Sep 6, 2024
View reviewed changes
Comment thread docs/api.rst Outdated
bdraco
bdraco reviewed Sep 6, 2024
View reviewed changes
Comment thread docs/api.rst Outdated
@bdraco bdraco changed the title Method to drop query keys Add new method without_query_params to drop query keys Sep 6, 2024
@bdraco bdraco changed the title Add new method without_query_params to drop query keys Add new method without_query_params to drop query keys Sep 6, 2024
webknjaz
webknjaz reviewed Sep 6, 2024
View reviewed changes
Comment thread docs/api.rst
@bdraco @webknjaz
Update docs/api.rst
e6cf2f2 
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
bdraco
bdraco reviewed Sep 6, 2024
View reviewed changes
Comment thread docs/api.rst
bdraco
bdraco reviewed Sep 6, 2024
View reviewed changes
Comment thread docs/api.rst Outdated
bdraco
bdraco reviewed Sep 6, 2024
View reviewed changes
Comment thread yarl/_url.py Outdated
bdraco
bdraco reviewed Sep 6, 2024
View reviewed changes
Comment thread CHANGES/898.feature.rst Outdated
bdraco added a commit that referenced this pull request Sep 6, 2024
@bdraco
Increment version to 1.10.0
c2d2147 
For new public feature #898
@bdraco bdraco mentioned this pull request Sep 6, 2024
Merged
bdraco
bdraco approved these changes Sep 6, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

Thanks @hongquan

@bdraco bdraco merged commit 2c531c7 into aio-libs:master Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@webknjaz webknjaz webknjaz left review comments

@bdraco bdraco bdraco approved these changes

@asvetlov asvetlov Awaiting requested review from asvetlov

@mjpieters mjpieters Awaiting requested review from mjpieters

+1 more reviewer

@dimaqq dimaqq dimaqq approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Ability to remove a specific query parameter Ability to remove a query value while retaining others

5 participants

@hongquan @mjpieters @webknjaz @dimaqq @bdraco