fix(ui-react): find thread datasets by mint_standard_variables (#94) - #95
Merged
Merged
Conversation
The thread wizard sent standard variable names to CKAN as a free-text `q`. CKAN does not index `mint_standard_variables`, and Solr splits the names on `_` and `~`, so the query matched prose rather than the annotation. Measured against TACC: the right datasets for 3 of 40 annotated variables, 28 returning zero and 3 returning only false positives. The Datasets step could not be satisfied, so the Runs step stayed locked. Match the field client-side instead, as the legacy Lit client does: - `packagesMatchingVariables` keeps the packages whose resources carry one of the requested variables, narrowing each package to those resources. The narrowing also fixes `datatype`, which is read off the first resource. - `loadDatasetResources` applies the same filter, so a selected dataset binds only the files the model input can read. - `searchAllPackages` pages past CKAN's 1000-row cap rather than truncating without saying so. TACC holds 215 packages, so this is one request. - `limit` now caps the datasets returned, not the pages fetched: the catalog has to be read in full before the filter can run. Verified live against ckan.tacc.utexas.edu: 40 of 40 annotated variables now return exactly the annotated datasets.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or 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
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.
Closes #94.
The defect
The thread wizard sent standard variable names to CKAN as a free-text
q. CKANdoes not index
mint_standard_variables, and Solr splits the names on_and~, so the query matched prose rather than the annotation.Re-measured against
ckan.tacc.utexas.edubefore the change: exactly the rightdatasets for 3 of 40 annotated variables. 28 returned zero. 3 returned only
false positives.
The Datasets step could not be satisfied, so the Runs step stayed locked. This
blocked #92.
The fix
Match the field client-side, as
ui/src/util/datacatalog/ckan-data-catalog.tsdoes.
packagesMatchingVariableskeeps packages whose resources carry one of therequested variables, and narrows each package to those resources. The
narrowing also corrects
datatype, which is read off the first resource.loadDatasetResourcesapplies the same filter. A dataset matches becausesome of its resources carry the variable, so binding all of them would hand
the model input files it cannot read.
searchAllPackagespages past CKAN's 1000-row cap instead of truncatingsilently. TACC holds 215 packages, so in practice this is one request.
limitnow caps the datasets returned, not the pages fetched: the catalog hasto be read in full before the filter can run.
mint_standard_variablesis a comma-separated string per resource at TACC. Alist is tolerated, and list entries are split again, because CKAN enforces no
shape.
Verification
Live against
ckan.tacc.utexas.edu, driving the app's ownfindDatasetsByVariablesfor every variable TACC annotates, against groundtruth read from the raw payload:
Offline: 791 tests pass. The new assertions were checked against the old
lookup - 8 of the 14 in
src/lib/__tests__/data-catalog.test.tsfail there,which is every behavioural one; the 6 that pass are the controls (bbox, dates,
no-variable pass-through).
Two existing fixtures had to change. They gave their packages no
mint_standard_variables, which only worked because the old code matched onprose - the fixtures encoded the bug.
Not fixed here
/datasets/searchin "Variable names" mode still searches CKAN free text. It isa browse surface, not the thread path, and how it should behave is a UX
question. Filed separately.