Support for enum mapping for slots with multiple enum ranges#31
Support for enum mapping for slots with multiple enum ranges#31martinwellman wants to merge 9 commits intomainfrom
Conversation
|
Should add unit tests. |
|
@martinwellman Thanks for working on this feature - supporting multiple enum ranges is a real need (linkml/linkml#2128). I'm sorry to have taken so long to get back to you on this. After reviewing, I have some concerns about the approach: Syntax Issue The PR uses range: [enum1, enum2] which isn't standard LinkML syntax. LinkML coerces this to a string "['enum1', The proper LinkML way to specify multiple enum ranges is any_of: slots: With this syntax, you'd access the enums via slot.any_of[*].range directly - no string parsing needed. Answers to your questions:
Recommendation I'd like to see a reworking of this to support the standard any_of syntax instead. The feature is definitely needed, but Would you be interested in updating the PR to use any_of, or would you prefer we take it from here? |
|
@amc-corey-cox Thanks for the response. I realized eventually that I should be using "any_of" instead of having "range" equal to an array. I have a newer working version locally that uses "any_of" that I've been using for our project. The problem is that there are other parts of LinkML-Map that would need to be updated as well to handle multiple enumerations for ranges. For example, the code for determining the reverse transformation as well as for making a graph of the transformations (there might have been others as well, I can't remember). Would you want these other parts to be updated to support "any_of" with this PR as well? There was also another problem I had to deal with related to "any_of", but I'll have to look back at my notes to recall what it was, since it's been a while. |
|
@martinwellman Thanks for your response. We'd really appreciate if you share the work that you have. Ideally, we'd update the other parts as well but if this implementation doesn't break things maybe we can get by without that; or maybe we can add that if you don't have time. Since you have a working use-case and understand this better, I'd prefer to start with what you've got then to come up with it on my own. If you don't have time or interest for that, I can take it from here. Just let me know. |
This PR adds support for mapping slots that have a range consisting of multiple enumerations.
Some important things to review:
Closes linkml/linkml#2128