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

Make VFS accept path-like objects to refer to files. #1818

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

thetorpedodog
Copy link
Contributor

When working on a change in tiledbsoma 1, we ran into somewhat mysterious error messages where things had worked before. This was because, unlike open (and other libraries which accept path-likes), tiledb.VFS only accepts str (and undocumented bytes) paths. Accepting path-like objects (like pathlib.Paths) lets us play better with other libraries and user code.

@nguyenv
Copy link
Collaborator

nguyenv commented Aug 22, 2023

Related to #1816.

Copy link
Collaborator

@nguyenv nguyenv left a comment

Choose a reason for hiding this comment

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

Looks good to me.

tiledb/vfs.py Outdated
from types import TracebackType
from typing import List, Optional, Type, Union

import tiledb.cc as lt

from .ctx import Config, Ctx, default_ctx

AnyPath = Union[str, bytes, os.PathLike]
Copy link
Collaborator

Choose a reason for hiding this comment

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

More of a note to self: We'll want to expand support pathlib.Path / path-like objects for other functions like tiledb.open, etc. eventually. OK to keep this typing definition in here for now but should probably be moved to a new _types.py in the future.

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 order to make this not look like it is intended to be used by any external users, I renamed this to _AnyPath, with no other changes.

Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

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

💯 thanks!

When working on a change in tiledbsoma [1], we ran into somewhat
mysterious error messages where things had worked before. This was
because, unlike `open` (and other libraries which accept path-likes),
`tiledb.VFS` only accepts `str` (and undocumented `bytes`) paths.
Accepting path-like objects (like `pathlib.Path`s) lets us play better
with other libraries and user code.

[1]: single-cell-data/TileDB-SOMA#1629
@thetorpedodog
Copy link
Contributor Author

I don’t have the power to merge changes, so one of you will have to do it once the tests go through.

@thetorpedodog thetorpedodog merged commit 486eb2e into dev Aug 23, 2023
@thetorpedodog thetorpedodog deleted the vfs-pathlib-path branch August 23, 2023 14:40
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

Successfully merging this pull request may close these issues.

3 participants