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

Integrate BioNLPProcessor with Odinson #392

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bgyori
Copy link

@bgyori bgyori commented Oct 18, 2023

This PR adds the BioNLPProcessor as one of the processor choices used for annotation. See clulab/reach#802.

WARNING: review for changing dependencies (this only works if Reach is available as a dependency) and changing default behavior (for illustrative purposes I changed application.conf to choose the BioNLPProcessor). Further changes may be needed to mitigate any unintended consequences of these.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@kwalcock
Copy link
Contributor

Thanks, @bgyori. I suspect that we might not want to incorporate this into the main branch, but it would be very nice to have the changes around for documentation or to serve as an example. Maybe we here can incorporate them into a branch for safe keeping. What does @myedibleenso who knows both reach and odinson think?

@@ -11,6 +11,7 @@ libraryDependencies ++= {
"ai.lum" %% "nxmlreader" % "0.1.2",
"org.clulab" %% "processors-main" % procVersion,
"org.clulab" %% "processors-corenlp" % procVersion,
"org.clulab" %% "reach-processors" % "1.6.3-SNAPSHOT"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to include a snapshot dependency in the default branch.

@kwalcock , is a version going to be published? What is the ETA on getting the transformer-based processor integrated into Reach?

Copy link
Contributor

Choose a reason for hiding this comment

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

Someone ;-) might still verify that the RC version works on aarch. Someone else might want to prepare a version of breeze/netlib that works on anything other than Linux (luhenry/netlib#21). I suspect that it will need to be me and that it will require forking that project and will take more time than we want.

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.

4 participants