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 nlp.py and add_sentiment() function #81

Merged
merged 7 commits into from
Jan 28, 2023
Merged

Conversation

bdfsaraiva
Copy link
Contributor

@bdfsaraiva bdfsaraiva commented Jan 17, 2023

First blood of #60

Note: need transformers in setup.cfg

chatminer/nlp.py Outdated
Comment on lines 12 to 14
sentiment_pipeline = pipeline("sentiment-analysis", model=model_path)
df["sentiment"] = [
sentiment["label"] for sentiment in sentiment_pipeline(list(df["message"]))
Copy link
Owner

Choose a reason for hiding this comment

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

can we make this more elegant and potentially improve performance by using pandas' apply?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, see if it's the way you want it, now.
Thanks for the tip.

@joweich
Copy link
Owner

joweich commented Jan 20, 2023

@bdfsaraiva is this PR in draft state for you or do you feel it should be merged already?

@bdfsaraiva
Copy link
Contributor Author

Hi, @joweich,

The package is yours, so the final decision will always be yours.
I think that, as with any feature, there is room for improvement, but that we already have a minimal usable version.

What is missing (as you had suggested) is to add an example in the readme file.

@joweich
Copy link
Owner

joweich commented Jan 22, 2023

What are ideas that you have in mind for improvement? We could add these ideas to a new feature issue and merge this PR for now 👍🏼
I plan to outsource the documentation into a dedicated website, so we don't need to put too much effort into the current readme.

@bdfsaraiva
Copy link
Contributor Author

bdfsaraiva commented Jan 23, 2023

What are you thinking for the documentation website? read the docs? Mkdocs? Other?

I can help you with that 🤙🏼

@joweich joweich merged commit b1daa4e into joweich:main Jan 28, 2023
@joweich
Copy link
Owner

joweich commented Jan 28, 2023

What are you thinking for the documentation website? read the docs? Mkdocs? Other?

I can help you with that 🤙🏼

@bdfsaraiva welcome to the main branch! Happy to have you as a contributor to this project! 🤗

I plan to go with readthedocs and sphinx as this is what I am used to. Will reach out once I get stuck!

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.

None yet

2 participants