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 implementation for relative_to and resolve #229

Closed
Gilthans opened this issue May 30, 2022 · 2 comments
Closed

Add implementation for relative_to and resolve #229

Gilthans opened this issue May 30, 2022 · 2 comments

Comments

@Gilthans
Copy link
Contributor

Hello!
Firstly - this is a wonderful library! I was about to implement something similar when I decided to look for it, and was delighted to see this. Such a clean, well designed project is hard to come by. Thanks to everyone involved.

I've seen most methods were implemented, and those that weren't had documentation on why. There are two methods I'd like to contest the decision though: relative_to and resolve.

  • resolve is stunted in a CloudPath since it is always absolute, but I'd argue it's better to return the same path instead of raising an error. This allows for using resolve to force into an absolute path, without knowing in advance if this is a cloud path or a local path.
  • relative_to I think should be implemented - relative_to is always used between two absolute paths, although the result is always a relative path. The result would be a PurePosixPath (and not a CloudPath), which after some thought does make sense.

I'd be happy to write a PR for this, if it would be accepted, and again thanks to all the contributors!

@Gilthans
Copy link
Contributor Author

After some thought, I'd argue the argument for resolve applies to absolute and as_posix as well, so I'd be happy to add those too.

@pjbull
Copy link
Member

pjbull commented May 30, 2022

Thanks @Gilthans, this has already come up for a few folks in #149, so I'm inclined to support it (especially since it makes writing Path/CloudPath agnostic code easier. I'm going to resolve this as a dupe of that bug and add your comments there. It also mentions is_relative_to, which is worth including as well.

I'll take a look at the PR and add a few notes shortly.

@pjbull pjbull closed this as not planned Won't fix, can't repro, duplicate, stale May 30, 2022
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

No branches or pull requests

2 participants