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

Pass manual=True to visualize_ner #27

Merged

Conversation

callistachang
Copy link
Contributor

Adjusted visualize_ner to include manual as an argument that is passed into displacy_render, and added a new file 03_visualize-ner-manual.py to show how it works.

Let me know if this PR can be improved, as this is one of my first open-source contributions, and I am still learning :)

Closes #14

@svlandeg svlandeg requested a review from Jette16 October 25, 2021 07:16
@Jette16
Copy link
Contributor

Jette16 commented Oct 28, 2021

Hello callistachang,

at first, thank you for your PR.

I would suggest two minor changes to it such that two possible errors are catched. Maybe you could catch these two things (e.g., using an if-else-statement) and post a streamlit warning instead such that the user understands the origin of the warning.

The first possible error occurs when the user sets manual to True but forgets to set show_table to False. Because the provided doc-Dictionary does not possess an attribute ents.
The other possible error occurs when manual is set to True but a Doc object is passed to it.

@svlandeg svlandeg removed the request for review from Jette16 November 2, 2021 16:06
@svlandeg
Copy link
Member

svlandeg commented Nov 2, 2021

Maintenance note: I'll put this PR in draft until we've been able to iron out the UX details. Feel free to move it back in "for review" once all comments are addressed!

@callistachang : If you have time to look into making these edits, feel free to add them directly to this PR. If you don't have time right now, let us know, and we can do them ourselves to make sure the PR gets into a state where we can merge it.

@svlandeg svlandeg marked this pull request as draft November 2, 2021 16:08
Error 1: When manual=True but show_table=True
Error 2: When manual=True but doc is not of type 'list'
@callistachang callistachang marked this pull request as ready for review November 7, 2021 16:28
@callistachang
Copy link
Contributor Author

Sorry for the delay, I must have missed the notifications! Hope this suffices for error checking

@svlandeg svlandeg requested a review from Jette16 November 8, 2021 09:11
@Jette16
Copy link
Contributor

Jette16 commented Nov 9, 2021

Great, thank you for adding the warning messages and also thank you again for your PR!

@svlandeg svlandeg merged commit 6eaaf43 into explosion:master Nov 9, 2021
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.

Passing manual=True to displacy.render
3 participants