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 gzipped jsonl reader #82

Closed
wants to merge 1 commit into from
Closed

Conversation

RXminuS
Copy link

@RXminuS RXminuS commented Jan 11, 2023

Add a simple modification to the jsonl reader that unzips a file. If the API instead of just allowing a path also allowed to pass in a file stream I could have handled it outside of this, but this seems the most straightforward solutions atm.

Add a simple modification to the jsonl reader that unzips a file. If the API instead of just allowing a path also allowed to pass in a file stream I could have handled it outside of this, but this seems the most straightforward solutions atm.
@rmitsch rmitsch added the enhancement New feature or request label Jan 13, 2023
@rmitsch
Copy link
Contributor

rmitsch commented Jan 13, 2023

Hi @RXminuS, thanks for your contribution! This would be a useful feature. Before considering this for merging, we'd like this PR to also:

  • implement a gzipped .jsonl writer, as it'd make more sense to have both
  • update the srsly docs w.r.t. the new functionality
  • add tests for the new functionality

Be aware that your current code is not valid Python. Let me know if you'd like to continue working on this! :-)

@rmitsch
Copy link
Contributor

rmitsch commented Feb 14, 2023

Implemented in #84.

@rmitsch rmitsch closed this Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants