Skip to content

Conversation

arsenetar
Copy link

Removing the need for the blobfile package when loading local files. Uses same approach as blobfile does for local file read. For local files this is generally not really providing any value as far as I can tell and introduces an additional dependency chain and logic around a simple file read.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

thanks, one comment

tiktoken/load.py Outdated
url = urllib.parse.urlparse(blobpath)
if url.scheme is None or url.scheme == "":
with open(blobpath, "rb") as f:
with io.BufferedReader(f) as br:
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you remove this extra io.BufferedReader? it's redundant

Copy link
Author

Choose a reason for hiding this comment

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

Removed.

@arsenetar arsenetar force-pushed the as/dont-use-blobfile-local branch 2 times, most recently from a1b1bee to 8a2e908 Compare September 2, 2025 14:16
Removing the need for the `blobfile` package when loading local files. Uses
same approach as `blobfile` does for local file read.
@arsenetar arsenetar force-pushed the as/dont-use-blobfile-local branch from 8a2e908 to 7a78a54 Compare September 2, 2025 14:18
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.

2 participants