Skip to content

chore: use urlunparse instead of string replace #2221

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

Merged
merged 1 commit into from
Aug 4, 2022
Merged

Conversation

JohnVillalovos
Copy link
Member

Use the urlunparse() function to reconstruct the URL without the
query parameters.

Use the `urlunparse()` function to reconstruct the URL without the
query parameters.
@nejch
Copy link
Member

nejch commented Aug 4, 2022

@JohnVillalovos this uses a private interface, see my comment at #2219 (comment).

@JohnVillalovos
Copy link
Member Author

@JohnVillalovos this uses a private interface, see my comment at #2219 (comment).

It is documented in the official docs, which to me means it is supposed to be used.

https://docs.python.org/3/library/urllib.parse.html

As is the case with all named tuples, the subclass has a few additional methods and attributes that are particularly useful. One such method is _replace(). The _replace() method will return a new ParseResult object replacing specified fields with new values.

@codecov-commenter
Copy link

Codecov Report

Merging #2221 (6d1b62d) into main (d263f57) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2221   +/-   ##
=======================================
  Coverage   95.45%   95.45%           
=======================================
  Files          81       81           
  Lines        5368     5368           
=======================================
  Hits         5124     5124           
  Misses        244      244           
Flag Coverage Δ
api_func_v4 81.35% <100.00%> (ø)
cli_func_v4 82.91% <100.00%> (-0.12%) ⬇️
unit 87.29% <100.00%> (ø)

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

Impacted Files Coverage Δ
gitlab/client.py 98.75% <100.00%> (ø)

@JohnVillalovos
Copy link
Member Author

JohnVillalovos commented Aug 4, 2022

My theory is that because the urllib.parse.ParseResult is a namedtuple they preface functions with underscores. I don't think it is required but maybe it is "best practice" 🤷‍♂️

The namedtuple documentation sort-of mentions that: https://docs.python.org/3/library/collections.html#collections.namedtuple

In addition to the methods inherited from tuples, named tuples support three additional methods and two attributes. To prevent conflicts with field names, the method and attribute names start with an underscore.

@nejch
Copy link
Member

nejch commented Aug 4, 2022

Ok, I will begrudgingly merge this 🐱

@nejch nejch merged commit 9e0b60f into main Aug 4, 2022
@nejch nejch deleted the jlvillal/unparse branch August 4, 2022 16:06
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