Adjustments for new productID inventory keyword by SolarDrew · Pull Request #549 · DKISTDC/dkist

SolarDrew · 2025-03-27T14:07:00Z

Closes #547

dkist/dataset/tiled_dataset.py

dkist/net/attrs.py

dkist/dataset/utils.py

SolarDrew · 2025-04-03T08:49:34Z

We should also regenerate the sample data as part of this PR so the productID gets included in the docstring.

Cadair · 2025-04-03T09:12:53Z

We should also regenerate the sample data as part of this PR so the productID gets included in the docstring.

It seems that not all the ASDF files have been regenerated to add it yet, so no point.

changelog/549.feature.rst

dkist/net/attr_walker.py

dkist/net/client.py

dkist/utils/inventory.py

Cadair · 2025-04-16T10:18:42Z

Poking about with search results I noticed that spectralLines is weird:

In [4]: Fido.search(a.Instrument.visp)["dkist"]["spectralLines"]
Out[4]: 
<QueryResponseColumn name='spectralLines' dtype='object' length=300>
                                           None
                                           None
                                           None
                                           None
                                           None
                                           None
                                           None
                                           None
                           ['Fe I (630.25 nm)']
                                           None
                                           None
                           ['Fe I (630.25 nm)']
                           ['Fe I (630.25 nm)']

Looks like we might need to do some post-processing here to make this not-terrible.

Cadair · 2025-04-16T10:19:45Z

Also we should probably add a test which fails if the return columns change so we know to update them. We shouldn't keep un-mapped ones around for long as it's a breaking change when we eventually map them.

This is kind of a hack but I'm not sure how else to do it because display_keys isn't exposed anywhere and if you set it in the class definition it just breaks.

Because it hasn't made its way into sample data inventory yet so it won't work

dkist/net/tests/test_attr_walker.py

docs/topic_guides/products_and_datasets.rst

eigenbrot · 2025-04-22T17:09:29Z

dkist/dataset/loader.py

    Files are stored in ...VISP_BKPLX
    <BLANKLINE>
+    This calibration has Dataset ID BKPLX.
+    The unique identifier for the input observe frames (Product ID) is (no ProductID).


is "(no ProductID)" a placeholder?

Any metadata ASDFs without a product Id in them will show this

eigenbrot · 2025-04-22T17:09:50Z

dkist/dataset/tiled_dataset.py

            consists of 9 frames.
            Files are stored in ...

+            This calibration has Dataset ID AJQWW and the original frames have Product ID ...


Is "..." a placeholder?

Ellipsis in doc tests ignore a section of the output for matching reasons. So in this case we ignore the path so the test isn't hard coded to a specific directory

docs/topic_guides/products_and_datasets.rst

SolarDrew

Couple of suggestions but otherwise I think this is good to go.

dkist/net/attrs.py

SolarDrew · 2025-04-23T09:52:13Z

docs/topic_guides/products_and_datasets.rst

+Dataset Status
+==============
+
+The status of a Dataset indicates if it's the latest calibration for that Product (``ACTIVE``), if it's an older calibration but still available (``DEPRECATED``) or no longer available (``REMOVED``).


May not be for this PR but I wonder if there's a way to inject this status into the dataset repr so that it's immediately obvious to users if they're using an old dataset.

yeah we need to do something around that.

Doing that would involve refreshing the dataset inventory record in the meta dict with the latest from the DC like we now do in .download().

Co-authored-by: Drew Leonard <[email protected]>

Cadair reviewed Apr 3, 2025

View reviewed changes

dkist/dataset/tiled_dataset.py Outdated Show resolved Hide resolved

Cadair reviewed Apr 3, 2025

View reviewed changes

dkist/net/attrs.py Outdated Show resolved Hide resolved

Cadair reviewed Apr 3, 2025

View reviewed changes

dkist/dataset/utils.py Outdated Show resolved Hide resolved

SolarDrew marked this pull request as ready for review April 3, 2025 09:05

Cadair reviewed Apr 3, 2025

View reviewed changes

changelog/549.feature.rst Outdated Show resolved Hide resolved

dkist/net/attr_walker.py Show resolved Hide resolved

dkist/net/client.py Show resolved Hide resolved

Cadair reviewed Apr 16, 2025

View reviewed changes

dkist/utils/inventory.py Show resolved Hide resolved

SolarDrew and others added 17 commits April 17, 2025 10:50

Start adding productId where it's needed

9ab6fbe

Update walker and test fixture

91535ea

Add productID to Dataset repr

cdde256

Changelog

b5df3a4

Brainfart

4131901

Make Fido response only show core keys

537f36a

This is kind of a hack but I'm not sure how else to do it because display_keys isn't exposed anywhere and if you set it in the class definition it just breaks.

Update example output in slice_tiles docstring

97f12ac

Update another docstring example

3cbe576

Minor corrections

4e35b7c

Remove actual product IDs from example outputs after all

a2a900a

Because it hasn't made its way into sample data inventory yet so it won't work

Add ability to search by status

2d3057e

Add debug logging to the client

ca475c5

Allow querying by all statuses

39bd467

cleanup old attrs and docstrings

8e5d3e6

Rename ProductID Product

c27d115

More docs

8de7cbe

Fix tests

a32748c

Cadair force-pushed the productid branch from 7d5867f to a32748c Compare April 17, 2025 15:15

Cadair added 2 commits April 17, 2025 16:41

Update dkist/dataset/utils.py

37bb976

Update changelog/549.feature.rst

cfe1768

Cadair reviewed Apr 19, 2025

View reviewed changes

dkist/net/tests/test_attr_walker.py Outdated Show resolved Hide resolved

Cadair force-pushed the productid branch from a19b086 to 06eb498 Compare April 22, 2025 16:01

Cadair added 4 commits April 22, 2025 17:13

Better annotations

072c9b3

Add some docs on Product ID and Status

14cf63c

Promote Status in return results if not active

11c851b

Fix dataset repr test

f6d1a76

Cadair force-pushed the productid branch from 06eb498 to f6d1a76 Compare April 22, 2025 16:14

krislarsnso reviewed Apr 22, 2025

View reviewed changes

docs/topic_guides/products_and_datasets.rst Outdated Show resolved Hide resolved

docs/topic_guides/products_and_datasets.rst Outdated Show resolved Hide resolved

eigenbrot reviewed Apr 22, 2025

View reviewed changes

Apply suggestions from code review

1bc01dd

Cadair reviewed Apr 23, 2025

View reviewed changes

docs/topic_guides/products_and_datasets.rst Outdated Show resolved Hide resolved

Update docs/topic_guides/products_and_datasets.rst

230b1b5

SolarDrew commented Apr 23, 2025

View reviewed changes

Apply suggestions from code review

1c75771

Co-authored-by: Drew Leonard <[email protected]>

Cadair approved these changes Apr 23, 2025

View reviewed changes

Cadair merged commit 0f24eee into DKISTDC:main Apr 23, 2025
25 of 26 checks passed

Conversation

SolarDrew commented Mar 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SolarDrew commented Apr 3, 2025

Uh oh!

Cadair commented Apr 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Cadair commented Apr 16, 2025

Uh oh!

Cadair commented Apr 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eigenbrot Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

Cadair Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

Cadair Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

eigenbrot Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

Cadair Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SolarDrew left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SolarDrew Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

Cadair Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants