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

BioThings PFOCR issues #203

Closed
colleenXu opened this issue May 29, 2024 · 15 comments
Closed

BioThings PFOCR issues #203

colleenXu opened this issue May 29, 2024 · 15 comments
Assignees
Labels
On Test Match https://github.com/biothings/biothings_explorer/labels

Comments

@colleenXu
Copy link

colleenXu commented May 29, 2024

Two things I've noticed:

(1) queries like https://biothings.transltr.io/pfocr/query?q=associatedWith.mentions.genes.ncbigene:5879 return no hits even when there are records with that value (example). Maybe it's because there's a misspelling in the metadata/fields entry - which says associatedWith.mentions.genes.bcbigene...

(2) For the PFOCR flavors, I'm not sure how to specify them in the OpenAPI spec / x-bte annotation. They're not regular query-level parameters like fields/size/jmespath/etc....can they be? (see previous Slack convo) @newgene

Refs:

@colleenXu colleenXu added the bug Something isn't working label May 29, 2024
@colleenXu
Copy link
Author

@newgene @everaldorodrigo I think the first point is easy to fix - and a bug that needs addressing...

I think the second point may need some more discussion

@everaldorodrigo
Copy link
Contributor

@colleenXu the first point fix was released to the CI, Test, and Production environments.

@everaldorodrigo
Copy link
Contributor

@colleenXu, according to Openapi specs we could modify the endpoint to accept a new parameter called flavor. Ex:

  • flavor=all
  • flavor=strict
  • flavor=synonymous

We also could keep both behaviors. The current one and the new one using a parameter called flavor.

What are your thoughts?

@newgene
Copy link
Member

newgene commented Jun 4, 2024

I would just use flavor=all|strict|synonymous, don't think we need to keep both.

@colleenXu
Copy link
Author

I think a new parameter like flavor sounds perfect!

@everaldorodrigo everaldorodrigo self-assigned this Jun 4, 2024
@everaldorodrigo everaldorodrigo added the On CI Match https://github.com/biothings/biothings_explorer/labels label Jun 4, 2024
@colleenXu
Copy link
Author

I have two observations for you/Alex Pico to check on. Otherwise, I think this looks fine and ready to deploy to all instances!

Two observations:

  • It looks like only the gene list differs for the 3 flavors (not chemicals or diseases). Is this correct @AlexanderPico?
  • The document order was different for the different flavors. I don't think this is important for Translator use, but I want to point it out in case you think it's a problem.

To check behavior, I took a few individual document/records and compared the mention counts. For example, I checked PMC6883319__gr8...

@colleenXu
Copy link
Author

colleenXu commented Jun 18, 2024

And a note to myself: for now, Andrew said I don't need to add this parameter's information to the BioThing API's SmartAPI annotation. We're currently using the default (all).

@AlexanderPico
Copy link
Contributor

It looks like only the gene list differs for the 3 flavors (not chemicals or diseases). Is this correct @AlexanderPico?

Correct

@colleenXu
Copy link
Author

@newgene
Copy link
Member

newgene commented Jun 25, 2024

order does not matter across different flavor parameter values, as they are different set of hits.

@colleenXu
Copy link
Author

So from my perspective, this seems good and ready to deploy to all instances.

@colleenXu
Copy link
Author

Let's use the tags to track deployment? Based on the Slack convos, it looks like this is on Test now...

@colleenXu colleenXu added On Test Match https://github.com/biothings/biothings_explorer/labels and removed On CI Match https://github.com/biothings/biothings_explorer/labels bug Something isn't working labels Jun 25, 2024
@colleenXu
Copy link
Author

@everaldorodrigo Are the PFOCR flavors already deployed to Prod? Does that mean this issue can be closed?

@everaldorodrigo
Copy link
Contributor

Hi @colleenXu, yes they were deployed to Prod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On Test Match https://github.com/biothings/biothings_explorer/labels
Projects
None yet
Development

No branches or pull requests

4 participants