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

Accept pathlib.Path as a valid path #1027

Merged
merged 1 commit into from
Dec 19, 2021
Merged

Accept pathlib.Path as a valid path #1027

merged 1 commit into from
Dec 19, 2021

Conversation

ltworf
Copy link
Contributor

@ltworf ltworf commented Jul 8, 2021

And also whatever can be converted to bytes.

Way more pythonic now!

@ltworf
Copy link
Contributor Author

ltworf commented Dec 18, 2021

Hello?

Plz?

@alex
Copy link
Member

alex commented Dec 18, 2021

I think this would be more appropriately handled with os.fspath.

@ltworf
Copy link
Contributor Author

ltworf commented Dec 19, 2021

Done

return s.encode(sys.getfilesystemencoding())
try:
strpath = os.fspath(s)
except Exception:
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to catch exceptions here? What case is that handling?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In [2]: os.fspath(1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-b00c080840d9> in <module>
----> 1 os.fspath(1)

TypeError: expected str, bytes or os.PathLike object, not int

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just because you seem to want to provide your own message. I'd be fine to not handle it and remove the else case later on as well.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think allowing the exception from os.fspath to propagate is fine.

And also whatever supports the protocol.

Way more pythonic now!
Copy link
Member

@alex alex left a comment

Choose a reason for hiding this comment

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

Thanks!

@alex alex merged commit 7120bb5 into pyca:main Dec 19, 2021
@ltworf ltworf deleted the Path_is_a_path branch December 19, 2021 18:39
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants