search

panosc-eu / search-api

PaN search api for WP3 and WP4
BSD 2-Clause "Simplified" License
4 stars 4 forks source link

Pan #61

Closed minottic closed 2 years ago

minottic commented 2 years ago

This PR should enable the use and results of the pan-ontologies-api microservice (https://github.com/ExPaNDS-eu/pan-ontologies-api).

To make it work, I had to add an extra field in the techniques collection (I renamed the old pid to id and added again pid which now contains the value from PaNET). This was needed as the filter on the a property which has the flag "id": true, doesn't seem to work in Loopback on a hasAndBelongsToMany relation, or at least I couldn't figure out how to make it work.

minottic commented 2 years ago

@nitrosx what do you think of the data model change? Do you know who is using that? I want to make sure it is not a disruptive change (as I renamed pid to id and added another pid which is not anymore the join key)

nitrosx commented 2 years ago

@minottic I'm not sure who is already using techniques at the moment. I have not heard of anybody doing that.

What would be the worst case scenario?

Possible solutions:

  1. we merge and be vigilant that we might need to step in and fix this on a short term notice
  2. we delay the merge, find if anybody is going to be impacted and act accordingly
  3. we delay the merge and find the solution to this issue, which could be being able to accept both id and pid somehow

I would go for 1, as this is fairly new. Then we could look into improving the pid management

thoughts?

minottic commented 2 years ago

@nitrosx ok. One easy solution could be not to rename pid to id and just add another field, let's say panetId. In this way there will be no possible backward compatibility problem. I can do that today if you agree

nitrosx commented 2 years ago

@minottic I like this solution. Let's go with this one.

One last question: what is exactly the pid and panetid and their relation? From your code, it looks like that what you call panetid(currently pid) is the URL of the technique on the PaNET site. If that's the case, I wonder if we can keep pid as it is and rename the second term URL or something meaningful.

minottic commented 2 years ago

@nitrosx yes, I can call it panetURL, no problem! For example the field ID in the link will be the content of panetURL (or how we call it) https://bioportal.bioontology.org/ontologies/PANET/?p=classes&conceptid=http%3A%2F%2Fpurl.org%2Fpan-science%2FPaNET%2FPaNET01184&jump_to_nav=true

nitrosx commented 2 years ago

@minottic : so that is the technique ID that you assigned to this technique on PaNET, but it is in the shape of URL. If you use that URL, it will take you to the file containing the technique definition (https://expands-eu.github.io/ExPaNDS-experimental-techniques-ontology/index-en.html#http://purl.org/pan-science/PaNET/PaNET01184) which assign to this specific field the name IRI.

So it is both an ID and a URL. Pick which one you like the best: panetURL or panetID.

minottic commented 2 years ago

@nitrosx here is the change

minottic commented 2 years ago

@nitrosx I have update the documentation. Apparently I cannot merge. Thanks for the help!

nitrosx commented 2 years ago

No worries. I do it