-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add data exploration GUI #200
Conversation
d2aa36f
to
9c0c36a
Compare
To have a play with the default DB:
You can pass a |
This is great!! OK, I loaded It says dbx.data I get ---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-13-e641cb6988d1> in <module>
----> 1 dbx.data
AttributeError: 'DatabaseExplorer' object has no attribute 'data' |
Btw, this PR definitely it should be definitely accompanied by a Data Exploration Tutorial in cosima-recipes |
The child It needs documentation for sure, and yes a tutorial would be on the To-Do list. There is a plan to have a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few thoughts as I read through. Looks great though, and definitely needed!
When first instantiated, or experiment changed, the variable | ||
selector widget needs to be refreshed | ||
""" | ||
self.de = DatabaseExtension(self.session, experiments=experiment_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this what you meant by having to re-cache the database contents when experiments are changed? Maybe DatabaseExtension
could cache the full thing once, and provide an interface to narrow its view to certain experiments (instead of recreation?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole extension thing is a horrible hack. I was hoping to re-implement most of the guts of it in the ORM layer if I had time, which I evidently do not, as this has taken WAY too long.
When I'm testing I make a single de
object and pass it into the widgets so never have to recalculate. I wasn't sure about always generating the whole thing every time, as it is possible to pass a list of experiments to it as well, and this is what happens when you just make an ExperimentExplorer
and pass it an experiment name. So that it is quick, as it just pulls in the information for the one experiment. It is important that it only has the info for a single experiment, as we don't want it picking up variables from other experiments.
The DatabaseExtension
thing is doing too much work. Like I said, hacky. For DatabaseExplorer
it makes the mapping between (name, long_name)
and experiment
for filtering. But we don't need that for the ExperimentExplorer
.
I should rethink that, but am happy for any input, as I've been staring at this stuff for too long and am a bit over it TBH.
I'm trying to write tests ATM, which should be useful if there is some refactoring done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the feedback too. Very helpful.
Removed redundant return_value_or_empty functions. Fixed bug with ExperimentExplorer __init__ when session not passed.
Changed to using a dict comprehension for formatting info output.
Modified test files for explore to ensure differences betweem two experiments to make tests meaninful.
Turns out the issue with widgets popping up where they shouldn't is because I am storing them in a jupyter-widgets/ipywidgets#2944 Python is weird sometimes. |
…ubwidgets were pointed to the same memory location in different instants. For more information see: jupyter-widgets/ipywidgets#2944 Removed unused imports.
Ok, random widgets turning up in weird places is now fixed. |
Enforced consistent style across classes.
I would like to get this out so people can use it. It would be nice to have more elegant SQL queries to replace the |
I'll have a quick play with getting the kind of data we want accessible at the ORM level, but I agree that it's better to have something usable rather than fuss over perfection! |
I have some ideas of how to change things, but I'm going to merge this first. If they come to fruition, they can come in through a new PR. |
ipywidgets for data exploration
Closes #199