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.

Sign up for GitHub

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

Jump to bottom

- #172

Closed
aogier opened this issue Nov 20, 2022 · 2 comments
Closed

- #172

aogier opened this issue Nov 20, 2022 · 2 comments
Assignees
@aogier
Labels
enhancement

Comments

@aogier
Copy link
Contributor

aogier commented Nov 20, 2022 •
edited
Loading

No description provided.

@jsvine
Copy link
Owner

jsvine commented Nov 22, 2022 •
edited
Loading

Hi @aogier, and thanks for opening this issue! I think adding an option to use multiprocessing is a solid suggestion. Although combine() does indeed work that way, there is one tricky part to make sure it works 100% faithfully, which is to ensure that the split between B and C is selected carefully, so that it occurs only at natural divisions in the corpus. (E.g., in between two sentences, in the case of a standard text.)

If you would like to submit a PR for the feature and relevant tests, I would welcome that. And if you have any questions or concerns about implementation before you submit, feel free to comment here or to contact me directly.

@aogier aogier mentioned this issue Nov 22, 2022
Closed
@jsvine
Copy link
Owner

jsvine commented Dec 5, 2022

@aogier Thanks for the nudge! I'll try to prioritize this and getting you feedback soon.

@aogier aogier closed this as completed Mar 13, 2023
@aogier aogier changed the title parallel processing - Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aogier aogier

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@aogier @jsvine