explosion / spacy-streamlit

👑 spaCy building blocks and visualizers for Streamlit apps
https://share.streamlit.io/ines/spacy-streamlit-demo/master/app.py
MIT License
806 stars 115 forks source link

Pass manual=True to visualize_ner #27

Closed callistachang closed 3 years ago

callistachang commented 3 years ago

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

Jette16 commented 3 years ago

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 commented 3 years ago

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.

callistachang commented 3 years ago

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

Jette16 commented 3 years ago

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