File refactor #102
File refactor #102
Conversation
Deploying datachain-documentation with
|
| Latest commit: |
9fcaf41
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://12e3091e.datachain-documentation.pages.dev |
| Branch Preview URL: | https://file-refactor.datachain-documentation.pages.dev |
|
The author of this PR, dberenbaum, is not an activated member of this organization on Codecov. |
| @@ -1288,7 +1288,7 @@ def export_files( | |||
|
|
|||
| def shuffle(self) -> "Self": | |||
| """Shuffle the rows of the chain deterministically.""" | |||
| return super().shuffle() | |||
| return self.order_by("sys.rand") | |||
| @@ -200,6 +200,11 @@ def open(self): | |||
| ) as f: | |||
| yield f | |||
|
|
|||
| def read(self): | |||
There was a problem hiding this comment.
Should this be read_text() and read_bytes()?
I'd expect read() to also support size= arg, similar to RawIOBase.read().
The API name also makes it unclear what it returns: bytes or str.
There was a problem hiding this comment.
I think it's a good idea to consider whether we can use a single File class that conforms with pathlib and uses more typical conventions like .open(mode="rb") and .read_text(). In that case we would need a separate method to read the data as an image, like file.read_image().
We would still need a way to tell other methods like to_pytorch() which read type to use or set a default read type. Not sure if it's feasible before release, but good to discuss it.
There was a problem hiding this comment.
I'm going to merge since I don't think your comments are a blocker or that this PR makes the situation any worse. We can keep discussing and follow up on top.
This is a bit close to release 😅 , but writing up the docs, the
Filehandling still feels slightly awkward. This makes a couple updates to hopefully clean up the API a bit:FileBasicsince it's redundant withFileand not used anywhere except inFileitself. I think it's enough to have the other classes inherit fromFile.get_value().read()is more familiar and they are somewhat redundant. Inpytorch(), check if it is aFiletype and useFile.read().Examples have been updated to reflect the latest API.
Let me know what you think.