-
Notifications
You must be signed in to change notification settings - Fork 56
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
Support subclasses #568
Merged
Merged
Support subclasses #568
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes intermine#508. There were three main problems: - subclass was added to any child class of the class (e.g. ThreePrimeUTR type constraint was added to Gene.UTRs.organism in addition to Gene.UTRs) - only direct subclasses were selectable, not indirect (descendants) - it was possible to create a subclass constraint to the original class (e.g. type constraint for Gene.UTRs to be UTR, which it already is)
- When expanding query tree to a constrainted subclass, dropdown will show correct value - Changing subclass will change it in query editor (when present) and fetch preview - Fix various incorrect docstrings - Fix bug creating multiple identical subclass constraints causing invalid query error - Remove stale subclasses (resulting in empty class in query editor) when removing last view of a class - Do not allow setting subclass constraint to the class itself (i.e. not a subclass)
Since you're able to set subclasses and drill down, without adding it to :im-query. Not sure if there's even done a full-path lookup in the tree, but it's still good practice to make sure the model supports it.
This led to a crash in the query builder caused by setting a subclass which was empty. The model should be kept complete, and trimming should be done in the UI.
Now we make sure to add all parent subclasses for an attribute.
- Clear subclass descendants from :enhance-query - Clear subclass descendant views from :order - Add parent subclasses when summarizing view
- Couldn't summarize unique class descendant of non-added subclass (Flymine e.g. Gene -> Child Features [Binding Site] -> Factor) - With im-path/walk fixes, subclass constraints changed contents of subclass dropdown (selecting dropdown item changed the dropdown) - With im-path/walk fixes, constrained subclass was displayed in parentheses instead of superclass - Class unique to parent subclass displayed a blank name in Query Editor (Flymine e.g. Gene -> Child Features [Binding Site] -> Factor)
Fixes intermine#550. I decided to keep the Summarise button, but add icons, tooltips and better names.
For this to work properly, tools that wish to support subclasses will have to make sure their query sets `from` to `imEntity.X.class`.
It used to crash due to lacking subclass support.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bluegenes hasn't been very accomodating of the existence of subclasses in the model (particularly when specified as type constraints) which this PR aims to resolve.
The class hierarchy is specified using the
extends
key for each class in the model. Indirect relationships are not included under this key, so a complete hierarchy would need to be parsed to properly determine subclasses (i.e. descendants of a class). This is a nested linear operation of all the classes in a model, but doesn't need to be recomputed except when changing mines. Therefore it makes sense to compute this hierarchy once when fetching the model, and caching it in app-db.Type constraints were previously present in the Query Builder, but only featured immediate subclasses and not indirect subclasses (the JSP webapp provides both). It also wasn't properly implemented which led to numerous bugs when building a query with type constraints, which have now been fixed.
Note that for subclasses to work correctly, a previous change removing empty classes from the model has been reversed, as otherwise a subclass lookup with
imcljs.path/walk
(depended on by many parts of the UI) would traverse nonexistent classes.In addition, there have been other improvements to the Query Builder:
Another aspect of supporting subclasses is having the Tool API display tools for subclasses of a class the tool supports. Details on this are documented in the
bluegenes.utils/suitable-entities
docstring. Tools that wish to support subclasses will have to make sure their query setsfrom
toimEntity.X.class
(whereX
is a class the tool supports, e.g.Gene
).This PR depends on changes to imcljs: intermine/imcljs#51
Finally, im-tables-3 will also need subclass support: intermine/im-tables-3#102
Fixes #459 fixes #508 fixes #522 fixes #539 fixes #550