nlpsandbox / nlpsandbox-controller

NLP Sandbox CWL workflow and tools
https://nlpsandbox.io
Apache License 2.0
3 stars 2 forks source link

Annotate Synapse submission entities with Tool information #77

Closed tschaffter closed 3 years ago

tschaffter commented 3 years ago

Is your proposal related to a problem?

I would like to choice the license of tools in the leaderboards. This will give an indication to the user before he/she attempts to pull the image. We could also use this information to pick a tool that we can feature with the PHI Deidentifier. We could also in the future create/filter leaderboards to only show tools with open licenses.

Describe the solution you'd like

During the evaluation workflow of a tool:

  1. Call /tool to get information about the tool (potentially /tool/dependencies too)
  2. Annotate the Synapse submission entity (consider using a prefix for all the properties of the Tool object)
thomasyu888 commented 3 years ago

@tschaffter, getting /tool/dependencies needs to be added to the client prior to being used.

The prefix would work except there are two fields in the Tool object that already have tool as the prefix (toolType, and toolApiVersion. What did you have in mind for a prefix?

You can see an example of this annotation being added if you scroll really to the right. What did want to show in the leaderboard?

tschaffter commented 3 years ago

The prefix would work except there are two fields in the Tool object that already have tool as the prefix (toolType, and toolApiVersion. What did you have in mind for a prefix?

I remember that this scenario crossed my mind. What about the prefix tool__ (two underscores)?

You can see an example of this annotation being added if you scroll really to the right.

This looks great!

What did want to show in the leaderboard?

thomasyu888 commented 3 years ago

Thanks @tschaffter , so the two conflicting fields will be tool__tool_api_version, tool__tool_type? That seems a bit clunky, alternatively, I could just append a prefix if tool doesn't already exist. I guess this is where I ask if we should actually have tool in the Tool schema - would it make sense for toolType to be type and toolApiVersion to be apiVersion?

tschaffter commented 3 years ago
  1. I agree with renaming toolType to type and toolApiVersion to apiVersion. In general I avoided to use the keyword type in schemas for no strong reason other that "type" may be a reserved keyword in some programming languages, which I tried to avoid. I can find APIs that meet with both approach. Exploring the API of GitHub lead me to accept the use of the "type".

  2. The proposed prefix "tool" would be a way to structure the annotations of a Synapse entity and conserve the hierarchy of information. For example Tool: { plop: { a = 1 }} could be encoded as the annotation `toolplop__a = 1`. Since these variable are not exposed directly to the frontend user, I think that it would be fine using double underscores.

github-actions[bot] commented 2 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.