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

Add URL.joinpath(*elements, encoding=False) to build a URL with multiple new path elements #704

Merged
merged 1 commit into from
Dec 14, 2022

Conversation

mjpieters
Copy link
Contributor

What do these changes do?

This adds a new method, URL.joinpath(), which creates a new URL object with
the arguments added as path elements:

>>> url = URL("http://example.com/foo")
>>> url.joinpath("bar", "ham", "spam/sub")
URL("http://example.com/foo/bar/ham/spam/sub")

The method accepts an encoded flag so you can pass in already-encoded path
elements.

Are there changes in behavior for the user?

There are no changes in behaviour for existing methods, but I did refactor
.__truediv__ to share code with the implementation for .joinpath().

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.

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Mar 29, 2022
@mjpieters mjpieters requested a review from asvetlov March 29, 2022 14:04
@codecov
Copy link

codecov bot commented Mar 29, 2022

Codecov Report

Merging #704 (dcced6d) into master (1a43a04) will not change coverage.
The diff coverage is 100.00%.

❗ Current head dcced6d differs from pull request most recent head 96afa11. Consider uploading reports for the commit 96afa11 to get more accurate results

@@           Coverage Diff           @@
##           master     #704   +/-   ##
=======================================
  Coverage   99.73%   99.73%           
=======================================
  Files           4        4           
  Lines         750      750           
  Branches      170      212   +42     
=======================================
  Hits          748      748           
  Misses          2        2           
Flag Coverage Δ
unit 99.60% <100.00%> (ø)

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

Impacted Files Coverage Δ
yarl/__init__.py 100.00% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@chey
Copy link

chey commented Jun 11, 2022

How different is this new method compared to using the following?

>>> url = URL("http://example.com/foo")
>>> url / "bar" / "ham" / "spam/sub"
URL('http://example.com/foo/bar/ham/spam/sub')

@mjpieters
Copy link
Contributor Author

mjpieters commented Jun 14, 2022

How different is this new method compared to using the following?

You can't use that expression with a dynamic list of path elements, not without having to use an explicit loop. Without this method you can't extend a series of URLs in a list comprehension, say:

>>> import random
>>> from yarl import URL
>>> urls = [URL("http://example.com/foo"), URL("http://example.com/bar")]
>>> names = ("terry_jones", "eric_idle", "john_cleese", "micheal_palin", "graham_chapman", "terry_gilliam")
>>> [url.joinpath("spam", *random.choices(names, k=random.randint(1, 3))) for url in urls]
[URL("http://example.com/foo/spam/micheal_palin"), URL("http://example.com/bar/spam/eric_idle/terry_gilliam")]

This is modelled on pathlib.Path.joinpath(), where Path() objects of course also support / as a joining operator.

docs/api.rst Outdated Show resolved Hide resolved
tests/test_url.py Outdated Show resolved Hide resolved
tests/test_url.py Outdated Show resolved Hide resolved
yarl/_url.py Outdated Show resolved Hide resolved
yarl/_url.py Outdated Show resolved Hide resolved
yarl/_url.py Outdated Show resolved Hide resolved
@webknjaz
Copy link
Member

webknjaz commented Aug 1, 2022

@mjpieters I've rebased the PR and it still passes but there's still a few things I'd like improved so I won't be merging it right now.

@mjpieters
Copy link
Contributor Author

@webknjaz all comments addressed now.

@asvetlov
Copy link
Member

The pull request looks good but merge conflict should be resolved first.

…ple new path elements

This is analogous to `Path(...).joinpath(...)`, except that it will not accept elements that start with `/`.
@mjpieters
Copy link
Contributor Author

mjpieters commented Dec 13, 2022

The pull request looks good but merge conflict should be resolved first.

Done! I'll merge it now that it's been green-lighted.

@mjpieters mjpieters merged commit 2dac93f into master Dec 14, 2022
@mjpieters mjpieters deleted the joinpath branch December 14, 2022 00:03
@asvetlov
Copy link
Member

Thanks @mjpieters !

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.

4 participants