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 support for 3.12 #149

Merged
merged 1 commit into from
Feb 14, 2024
Merged

add support for 3.12 #149

merged 1 commit into from
Feb 14, 2024

Conversation

skshetry
Copy link
Member

@skshetry skshetry commented Feb 8, 2024

@codecov-commenter
Copy link

codecov-commenter commented Feb 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6567d50) 84.15% compared to head (01c2e05) 84.15%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #149   +/-   ##
=======================================
  Coverage   84.15%   84.15%           
=======================================
  Files          12       12           
  Lines         385      385           
  Branches       51       50    -1     
=======================================
  Hits          324      324           
  Misses         52       52           
  Partials        9        9           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines 272 to 273
_ = str(path) # force Path to compute `Path._str`
path._str = path._str.rstrip("/") # remove trailing slash
Copy link
Member Author

Choose a reason for hiding this comment

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

upath now returns a new instance of accessor each time you access those. Also, UPath._accessor usage is now deprecated, although it is what is used internally.

I haven't found a correct place to fix this without extending, as
Upath.path are properties that just ask Pathlib.path which in turn just calls __str__ (which is adding a trailing slash to it).

Ideally, gcfs should support a path with a trailing slash, but Upath should not add a trailing slash when it's not provided either. I'll try to create an issue on both repositories.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @skshetry, can you add a note here pointing to those issues?

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, looking at s3fs, it does not handle s3fs.info("bucket/") or s3fs.info("bucket") either. It raises FileNotFoundError in both cases.

However, it does magically work with _exists() due to a separate implementation.
As we already do .mkdir(), the bucket is created. So I don't think we should try to support something that is not supported by gcsfs at all.

The users of this library need not worry about whether the bucket is created or not. I'm skipping this ugly workaround for now.

@skshetry skshetry linked an issue Feb 14, 2024 that may be closed by this pull request
requires `universal-pathlib>=0.2.0`
@skshetry skshetry merged commit 2a117c6 into main Feb 14, 2024
16 checks passed
@skshetry skshetry deleted the 3.12 branch February 14, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3.12 compatibility
3 participants