DM-45429: Initial implementation of a general query result

[andy-slac]

Registry method queryDatasetAssociations is reimplemented (for both
direct and remote butler) to use new query system and new general query
result class.

Checklist

• ran Jenkins
• added a release note for user-visible changes to doc/changes
• (if changing dimensions.yaml) make a copy of dimensions.yaml in configs/old_dimensions

[andy-slac]

@TallJimbo, I have an initial version of a general query result calss and I reimplemented queryDatasetAssociations to use that to make sure it can work. Could you check that what I've done so far matches your model for it?

[codecov]

Codecov Report

Attention: Patch coverage is 92.88136% with 21 lines in your changes missing coverage. Please review.

Project coverage is 89.48%. Comparing base (20b5ab1) to head (5f358c5).

Files	Patch %	Lines
python/lsst/daf/butler/column_spec.py	77.77%	8 Missing and 4 partials ⚠️
python/lsst/daf/butler/queries/_query.py	77.27%	3 Missing and 2 partials ⚠️
...hon/lsst/daf/butler/direct_query_driver/_driver.py	50.00%	0 Missing and 1 partial ⚠️
.../lsst/daf/butler/queries/_general_query_results.py	98.21%	1 Missing ⚠️
python/lsst/daf/butler/remote_butler/_registry.py	87.50%	0 Missing and 1 partial ⚠️
...ote_butler/server/handlers/_query_serialization.py	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1047      +/-   ##
==========================================
+ Coverage   89.44%   89.48%   +0.04%     
==========================================
  Files         360      361       +1     
  Lines       45842    46095     +253     
  Branches     9406     9454      +48     
==========================================
+ Hits        41004    41249     +245     
- Misses       3506     3513       +7     
- Partials     1332     1333       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TallJimbo]

As I mentioned in the stand-up, I had imagined adding a to-be-public Query.general method rather than Query.dataset_associations, as sort of a power-user escape valve. I'm not opposed to also having Query.dataset_associations.

[TallJimbo]

There's one place in QG generation where we'd want a method sort of like this that yields tuples of DatasetRef and DataCoordinate (it currently calls DataCoordinateQueryResults.findRelatedDatasets).

I had originally planned to do that via a special dict-like row object that had single-row get_ref, get_data_id etc. methods, but I think I like your iteration adapter better as it allows for doing work common to all rows up front. But I'd like to extend this to be able to support at least DatasetRef + DataCoordinate + [extra stuff] somehow, and maybe even support DatassetRef objects for multiple dataset. Maybe something like this?

def iter_tuples(self, *dataset_type: DatasetType) -> Iterator[tuple[DataCoordinate, dict[str, DatasetRef], dict[ResultColumn, Any]]

where it always returns the full data ID, a dict of DatasetRef keyed by dataset type name, and then whatever columns aren't included in one of those? To be honest I'm not thrilled with this, either, but maybe it'll inspire you to find something better?

[andy-slac]

The return annotation is hard to read. Would it make sense to introduce a helper NamedTuple like this:

class GeneralResultTuple(typing.NamedTuple):

    data_id: DataCoordinate
    refs: dict[str, DtasetRef]  # or maybe just list[DatasetRef]?
    raw_row: dict[str, Any]

class GeneralQueryResults(QueryResultsBase):

    def iter_tuples(self, *dataset_type: DatasetType) -> Iterator[GeneralResultTuple]:
        ...

[TallJimbo]

👍 I do like NamedTuple for this; you can still unpack it if you want to, but it's also very clear what's in it and you can hang documentation on it. And I like list rather than dict for the DatasetRefs, too, since we can make that ordering consistent with the argument rather than arbitrary.

[TallJimbo]

Another possibility for the key type here is a <dimension-or-dataset-type-name>.<column> string; that might be better if we make this type more publicly accessible. In either case it should probably match however we let the user specify the desired result columns in any public interface that returns one of these.

[andy-slac]

I switched to str as a key, it is indeed more natural as API uses strings as parameters. I also replaced dataset_associations with general which takes the same parameters as GeneralResultSpec.

[TallJimbo]

Since we don't need to mutate them, better to make the value types collections.abc.Set rather than set.

What do you think about making all of these keyword-only arguments, and adding a *args: str variadic argument that we'd filter though interpret_identifier (in queries/_identifiers.py) and use to update the existing categorized values?

It might also be nice to have a simple way to say "all of the fields necessary for DatasetRefs of this dataset type", maybe datasets={name: ...} or something?

[andy-slac]

@timj, this is how replacement code for queryDatasetAssociations is going to look like. I checked RepoExportContext class to see if I can replace queryDatasetAssociations usage there, but that class does not even have butler instance now, and it uses registry for everything. We can do it later and keep queryDatasetAssociations in registry for now. Both direct and remote registries use new query system.

[timj]

Great. I had forgotten that we need to change the export code so that it doesn't use registry.