clulab / reach

Reach Biomedical Information Extraction
Other
97 stars 39 forks source link

Remove references to separate HGNC resource #728

Closed bgyori closed 3 years ago

bgyori commented 3 years ago

This corresponds to https://github.com/clulab/bioresources/pull/56 and should be tested/merged once https://github.com/clulab/bioresources/pull/56 is merged and released. It removes references to a separate HGNC resource file.

MihaiSurdeanu commented 3 years ago

@kwalcock : this should be merged once the latest PR in bioresources is released. Thanks!

Also, I am discussing with @enoriega to merge bioresources as a subproject into Reach, so we don't go through this dance everytime.

kwalcock commented 3 years ago

Yes, as soon as the previous PR passes its checks.

kwalcock commented 3 years ago

@bgyori, if I merge master into this and then run the tests with the locally published hgnc_up branch of bioresources (straight or merged with its master branch), there is one test failure. Perhaps there is an interaction with the previous PR that was merged in the meantime.

[info] - should match expected grounding IDs in the text p53, RAC1, RhoA, ROCK1, and SAF-1 GGPs. *** FAILED *** (11 milliseconds)
[info]   false was not true (TestOverrides.scala:188)
bgyori commented 3 years ago

Yes, I noticed that test failure locally but when I pasted the sentence into the shell, I got all the right groundings so I wasn't sure what to make of it. I was having trouble debugging in IntelliJ (sent a separate email about that) so couldn't figure out what the specific issue was. Do you have a sense of which test condition is failing?

kwalcock commented 3 years ago

Now I have the same IntelliJ problem. There was no sbt tab on the right side of the screen, which means IntelliJ doesn't think it's an sbt-built program and probably ignores dependencies. I think that it has to do with the .bsp directory which newer versions of sbt seem to be using. I erased that and also .idea and the tab reappeared [after I restarted IntelliJ].

kwalcock commented 3 years ago

It looks like grounding P07998 is failing.

bgyori commented 3 years ago

I see, I can reproduce that now (and IntelliJ is working again after "Reimport All sbt Projects"), it looks like there is an ambiguity for RAC1

RAC1»···P07998»·Homo sapiens
RAC1»···P63000»·Homo sapiens

and for some reason the wrong one is chosen now. I'll have to add an override in bioresources for this one.

kwalcock commented 3 years ago

I'm glad you know what to do. I'll get the hang of it eventually.

bgyori commented 3 years ago

Okay, I pushed the change to bioresources and the test passes for me now.