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

Combine models with retain_original=False, and docs #78

Merged
merged 2 commits into from
Sep 27, 2017

Conversation

thallada
Copy link
Contributor

Thanks for making this library, it's really great and easy to use. I was trying to load a large corpus file-by-file and noticed an error when trying to use markovify.combine on models with retain_original=False:

AttributeError: 'Text' object has no attribute 'parsed_sentences'

I think this change fixes the issue.

And, I documented how to create models file-by-file and combine them into one model in the README. I was wondering how to do that when I first started using markovify.

Also, kind of unrelated, but I added a tip about using spaCy to the README too.

Also, document how to create models file-by-file and combine them into one
model.
@coveralls
Copy link

coveralls commented Sep 24, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 9a6f14d on thallada:combine-retain-original-false into 4d62bfb on jsvine:master.

@thallada thallada changed the title Combine retain original false Combine models with retain_original=False, and docs Sep 24, 2017
@jsvine
Copy link
Owner

jsvine commented Sep 27, 2017

Thank you, Tyler! Great catch and great fix. Much appreciated.

@jsvine jsvine merged commit 8db3b9c into jsvine:master Sep 27, 2017
@jsvine
Copy link
Owner

jsvine commented Sep 27, 2017

Merged, and now available in 0.6.3.

(And nice tip about spaCy.)

@thallada thallada deleted the combine-retain-original-false branch September 27, 2017 22:03
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