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

feat(bindings/python): let path can be PathLike #5636

Merged
merged 9 commits into from
Feb 18, 2025

Conversation

asukaminato0721
Copy link
Contributor

Which issue does this PR close?

Closes #5571.

Rationale for this change

allow Path in py code.

What changes are included in this PR?

change input type

Are there any user-facing changes?

user will be allowed to directly use Path

@asukaminato0721 asukaminato0721 changed the title init feat(bindings/python): let path can be PathLike Feb 18, 2025
@asukaminato0721
Copy link
Contributor Author

asukaminato0721 commented Feb 18, 2025

ok, I find the problem.

async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {

the origin crate use &str so

  1. change the whole crate to use AsRef<Path>, too many changes.
  2. do the type change on python binding, I prefer this.

@asukaminato0721 asukaminato0721 marked this pull request as ready for review February 18, 2025 08:26
@Xuanwo
Copy link
Member

Xuanwo commented Feb 18, 2025

Thank you @asukaminato0721 for this great work! Would you like to add some tests to the python code to ensure it works as expected?

@asukaminato0721
Copy link
Contributor Author

@Xuanwo added

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Love it, thank you!

@Xuanwo Xuanwo merged commit a7b1c3c into apache:main Feb 18, 2025
180 checks passed
@asukaminato0721 asukaminato0721 deleted the python-pathbuf branch February 18, 2025 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

new feature: read,write maybe need support pathlib Path
2 participants