Fix ipyoptimade version
#703
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #703 +/- ##
=======================================
Coverage 84.29% 84.29%
=======================================
Files 18 18
Lines 3584 3584
=======================================
Hits 3021 3021
Misses 563 563
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
danielhollas
left a comment
danielhollas
left a comment
There was a problem hiding this comment.
Thanks! Leaving review up to @superstar54 since I am not using Optimade in my app and have no experience with it.
| provider_database_groupings=kwargs.pop( | ||
| "provider_database_groupings", | ||
| default_parameters.PROVIDER_DATABASE_GROUPINGS, | ||
| ), |
There was a problem hiding this comment.
Does this affect any functionality? Was this used in QeApp?
There was a problem hiding this comment.
Not sure what the provide_database_groupings argument was used for. Maybe @eimrek can comment on its removal in ipyoptimade v1.0.0. As for the widget itself, it is used in the QE app as one of the structure importers. Recent changes in MC (again, I'll let @eimrek comment here) rendered ipyoptimade<1.0.0 incompatible with the MC database.
There was a problem hiding this comment.
I was more asking about whether provider_database_groupings argument is used in QeApp (or elsewhere).
There was a problem hiding this comment.
provider_database_groupings was not made publicly accessible by ipyoptimade. For info, see the relevant PR - aiidalab/ipyoptimade#21
There was a problem hiding this comment.
IIRC, provide_database_groupings was used to separate our materials cloud optimade DBs into two groups: main and contributed. I think I had a problem with it at some point and it made sense to just remove it.
|
Looking at aiidalab/ipyoptimade#21 actually reminded me why we originally could not easily upgrade ipyoptimade --- that PR bumped the minimum ipywidgets version to 8.1, and we do not yet support this version (see in-progress PR #644) . However, it seems that support for earlier ipywidget version was added back in aiidalab/ipyoptimade@2876092 and released in version 1.2.0. So I think the minimum version here should be specified as |
Actually, looks like we need to set the minimum version to |
edan-bainglass
force-pushed
the
fix-optimade
branch
from
b0ad60d to
a1643fd
Compare
superstar54
left a comment
superstar54
left a comment
There was a problem hiding this comment.
Hi all, I tested this PR in the latest QE app image. Select the database, click elements on the periodic table, and search; the widget works.
|
@danielhollas release? |
|
Sure, go ahead! Master seems to contain mostly bug fixes so 2.4.1 seems appropriate? |
Recent changes to Materials Cloud and/or the OPTIMADE client have broken the
OptimadeQueryWidget. This PR updates theipyoptimadedependency and fixes the use of the package in the widget, restoring access to the Materials Cloud database.