-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add Pathlib #1840
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 Pathlib #1840
Conversation
I did implement TryFromObject for CString recently, but you could also just have a nulcheck function that makes sure a string doesn't have a nul char in it, and returns a ValueError if it does, then just call |
The problem is that CPython support os.stat("/dev/null\x00") Should not throw an error. |
Ah, I see; that is tricky. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems only trailing \x00
exists in tests. Trimming them before making PyPathLike
would work?
This is not specifically related to this PR, but while reading, i found i have no idea why PyPathLike
is not PyPath
but Like
This will actually give the opposite result. As
Thats how they are called in CPython. |
I try to test this problem a bit, but I can't make null-included path working. Could you describe your working environment? macOS, python 3.8.1, APFS
Linux, Python 3.5.2, ext4
|
If you will look at the tests. For example https://github.com/RustPython/RustPython/pull/1840/files#diff-6bc3eb7fdef7557c945c66fb0b60a134R1921 |
@coolreader18 @youknowone can this be pushed and we will solve the |
It seems |
I am not sure I understand... Would you like to change the |
I guess so. I didn't check cpython implementation well yet, but it seems it is more close to |
@palaviv I think fixing it later would be fine, it's not too critical to pathlib, and I think it would rarely happen in practice. |
I have merged this and we can do the proposed changes in the future |
I checked about PyPathLike impl but it revealed enum is not fit for the use case. |
This change add
pathlib
and changes manyos
method to support path like objects.Many of the tests fail because Rust does not support
Cstring
withnul
. Not sure how to bypass that. Would appreciate suggestion on how to solve this.