-
Notifications
You must be signed in to change notification settings - Fork 6
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
Use datalad-nexts constraints system #401
base: main
Are you sure you want to change the base?
Conversation
916d29f
to
4e949e2
Compare
I have applied a few minimal fixes to this PR such that the CI now passes. I'm not sure, however, if it is in a state you would like it to be in @mih given that there were quite a few developments in datalad-next's constraint system in the meantime. |
Revisiting this PR with DataLad 0.18.5 and DataLad-next 1.0.0b3, I can confirm that tests pass locally for me. I also did some basic clicking around. With this PR branch checked out, right-click to get file content #406 is indeed fixed (as stated previously). However, new weirdness seems to appear. All inputs become text widgets, and there is an odd default for a list of paths. This is when doing hen doing right-click (on a not installed subdataset) -> Dataset commands -> Get content ... for this PR: ... compared to current I was not able to dig deeper. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #401 +/- ##
==========================================
- Coverage 60.44% 57.05% -3.39%
==========================================
Files 49 48 -1
Lines 3504 3244 -260
==========================================
- Hits 2118 1851 -267
- Misses 1386 1393 +7 ☔ View full report in Codecov by Sentry. |
I can reproduce the issue @mslw reported. I believe that the reason for the failure to select the correct Input Widget stems from the fact that the
Here is a patch with a fix: diff --git a/datalad_gooey/constraints.py b/datalad_gooey/constraints.py
index 71cc889..46c85e9 100644
--- a/datalad_gooey/constraints.py
+++ b/datalad_gooey/constraints.py
@@ -22,6 +22,7 @@ from datalad_next.constraints import (
NoConstraint,
)
from datalad_next.constraints.parameter_legacy import EnsureParameterConstraint
+from datalad_next.constraints.compound import ConstraintWithPassthrough
from datalad.distribution.dataset import EnsureDataset as CoreEnsureDataset
from datalad.distribution.dataset import (
Dataset,
diff --git a/datalad_gooey/param_form_utils.py b/datalad_gooey/param_form_utils.py
index edfccb8..d4fcbc6 100644
--- a/datalad_gooey/param_form_utils.py
+++ b/datalad_gooey/param_form_utils.py
@@ -32,6 +32,7 @@ from .api_utils import (
from .utils import _NoValue
from .constraints import (
AltConstraints,
+ ConstraintWithPassthrough,
EnsureBool,
EnsureExistingDirectory,
EnsureDatasetSiblingName,
@@ -212,6 +213,12 @@ def _get_parameter(
# if we have no idea, use a simple line edit
type_widget = pw.StrParameter
+
+ if isinstance(constraint, ConstraintWithPassthrough):
+ # to make proper constraint identification below work, get the
+ # internal constraint:
+ constraint = constraint.constraint
+
### now some parameters where we can derive semantics from their name
if isinstance(constraint, EnsureDataset) \
or isinstance(constraint, EnsureExistingDirectory): |
DataLad-next constraints were (in all cases?) ConstraintWithPassthroughs that slipped by the widget-to-constraint detection. This change extracts the internal constraint if its within a ConstrainWithPassthrough to make it functional again.
Coming with datalad/datalad-next#116
Shrinks gooey code base by 5%.
Tests cannot pass yet. Waiting for datalad-next 0.7.