Populate map principal data CATDESC from descriptor by jtniehof · Pull Request #2712 · IMAP-Science-Operations-Center/imap_processing

jtniehof · 2026-02-17T20:04:48Z

Change Summary

Overview

The CATDESC (and thus plot title in CAVA) for all ENA maps is "Mono-energetic ENA intensity". This doesn't provide much detail on what intensity is being shown, and the IMWG has requested that this be updated with a human-readable description of the map descriptor.

File changes

ena_maps/utils/naming.py: add function to parse a CATDESC from a MapDescriptor
tests/ena_maps/test_naming.py: tests for above
ena_maps/ena_maps.py: patch the CATDESC in build_cdf_dataset
tests/ena_maps/test_ena_maps.py: update tests for above

Testing

New test was added for MapDescriptor.to_catdesc; the test for build_cdf_dataset was updated to check the CATDESC was properly populated. build_cdf_dataset now requires a fairly well-constructed descriptor, so some tests were updated to provide that.

Other notes

This PR is in draft while the IMWG reviews the actual contents of the CATDESC; there shouldn't be structural changes after that.

This does create a circular import between ena_maps and ena_maps.utils.naming. It doesn't seem to cause problems but I still don't like it.

There are several cases where a malformed map descriptor will cause a regex to fail and horrible exceptions to be thrown. I figure the implicit contract that deep in the code is that the descriptor is well-formed, and I usually avoid catching an exception just to throw another exception. Open to alternate approaches.

* Put species before the quantity * Spell out Combined (and space out from instrument name) * Special-case ISN [species] Rate

subagonsouth

Saw this draft PR and thought I would give some feedback.

subagonsouth · 2026-02-18T17:16:11Z

imap_processing/ena_maps/utils/naming.py

+        instrument_names = {
+            "l": "Lo",
+            "t": "Lo",
+            "ilo": "Lo",
+            "h": "Hi",
+            "u": "Ultra",
+            "idx": "IDEX",
+            "glx": "GLOWS",
+        }


There is some overlap here with the MappableInstrumentShortName enum class. I wonder of we could leverage that somehow. The two issues that would need to be sorted are:

trimming the full Lo names

convert Lo, Hi, Ultra to CamelCase but keep IDEX, GLOWS as is.

https://github.com/jtniehof/imap_processing/blob/0f6de5e32ad425b7c4fab46ccc86918bd578cc52/imap_processing/ena_maps/utils/naming.py#L18

One idea would be to modify the class such that the value attribute is a named tuple so that you could do something like:

instrument_long = self.instrument.long_name instrument_short = self.instrument.short_name

I looked a bit at that class and was hesistant to do much changing because the CATDESC names are fairly specific to this task, and aren't new generic names. Your comment here and below encouraged me to take another stab at it and I think the results, while still hacky, are at least shorter and easier to understand.

subagonsouth · 2026-02-18T17:19:27Z

imap_processing/ena_maps/utils/naming.py

+        instrument = next(
+            (
+                name
+                for desc, name in instrument_names.items()
+                if self.instrument_descriptor.startswith(desc)
+            )
+        )


I think this could be reversed to use the mapping and instrument attribute:

Suggested change

instrument = next(

(

name

for desc, name in instrument_names.items()

if self.instrument_descriptor.startswith(desc)

)

)

instrument = instrument_names[self.instrument.value]

Wrapping this up in using self.instrument instead of the descriptor made this whole mess go away.

subagonsouth · 2026-02-18T17:37:00Z

imap_processing/ena_maps/utils/naming.py

+        species = self.species.title()
+        if species == "Uv":
+            species = "UV"


Minor nit... a ternary operator should be used here.

Suggested change

species = self.species.title()

if species == "Uv":

species = "UV"

species = "UV" if self.species == "uv" else self.species.title()

I am deeply ashamed that I missed a chance to use my favorite operator.

subagonsouth · 2026-02-18T17:47:52Z

imap_processing/ena_maps/ena_maps.py

+        # And CATDESC for principal data
+        possible_principal_data = (
+            "dust_rate",
+            "ena_intensity",
+            "ena_spectral_index",
+            "glows_rate",
+            "isn_rate",
+            "isn_rate_bg_subtracted",
+        )
+        for principal_data in possible_principal_data:
+            if principal_data in cdf_ds:
+                catdesc = naming.MapDescriptor.from_string(descriptor).to_catdesc()
+                cdf_ds[principal_data].attrs["CATDESC"] = catdesc
+                # Only one principal variable
+                break


It seems like it would be cleaner to return a tuple form to_catdesc() consisting of the variable name and the catdesc or add a principal_data() property method so that you could just do:

principal_var, catdesc = naming.MapDescriptor.from_string(descriptor).to_catdesc() cdf_ds[principal_var].attrs["CATDESC"] = catdesc

or

map_desc = naming.MapDescriptor.from_string(descriptor) cdf_ds[map_desc.principal_data].attrs["CATDESC"] = map_desc.to_catdesc()

Yes, a property on MapDescriptor is a lot better...I was already nervous about the fact I was copy-pasting this into the L3 code, and the Ultra situation made it even worse. See if the new approach makes sense.

subagonsouth · 2026-02-18T17:53:54Z

imap_processing/ena_maps/ena_maps.py

            {"DELTA_PLUS_VAR": "epoch_delta", "BIN_LOCATION": 0}
        )

+        # And CATDESC for principal data


Please note that Ultra does not use this build_cdf_dataset() method currently. I think there is an intent to move this method to the generic SkyMap class and transition Ultra to using that, but you will need to dig through the Ultra code to see where you can fit this in for updating the Ultra CATDESC.

Thanks for that catch! I was feeling a little nervous since it wasn't dealing with HEALPix.

I wound up making a complete copy of the main ultra_l2 test because it works quite differently between using the output_map_structure vs. the descriptor--the descriptor can't specify ECLIPJ2000 and it also casts the pixel size to a float somewhere along the way. That said, now I see there was another test using the descriptor so going to go move into that test now....done. I can rebase out that misstep if preferred.

subagonsouth · 2026-02-18T17:58:54Z

imap_processing/tests/ena_maps/test_naming.py

+
+    def test_to_catdesc(self):
+        cases = [
+            (
+                "h45-spx-h-hf-sp-ram-hae-4deg-3mo",
+                "IMAP Hi45 H Spectral, HAE Helio Frame, Surv Corr, Ram, 4 deg, 3 Mon",
+            ),
+            (
+                "h45-spx0305-h-hf-sp-ram-hae-4deg-3mo",
+                "IMAP Hi45 H Spectral, HAE Helio Frame, Surv Corr, Ram, 4 deg, 3 Mon",
+            ),
+            (
+                "hic-ena-h-hf-sp-ram-hae-4deg-3mo",
+                "IMAP Hi Combined H Inten, HAE Helio Frame, Surv Corr, Ram,"
+                " 4 deg, 3 Mon",
+            ),


This would be better suited as a parameterized test. For example:

@pytest.mark.parametrize( "map_desc, expected_catdesc", [ ( "h45-spx-h-hf-sp-ram-hae-4deg-3mo", "IMAP Hi45 H Spectral, HAE Helio Frame, Surv Corr, Ram, 4 deg, 3 Mon", ), ( "h45-spx0305-h-hf-sp-ram-hae-4deg-3mo", "IMAP Hi45 H Spectral, HAE Helio Frame, Surv Corr, Ram, 4 deg, 3 Mon", ), ( "hic-ena-h-hf-sp-ram-hae-4deg-3mo", "IMAP Hi Combined H Inten, HAE Helio Frame, Surv Corr, Ram," " 4 deg, 3 Mon", ), ], ) def test_to_catdesc(self, map_desc, expected_catdesc): md = MapDescriptor.from_string(descriptor_str) actual_catdesc = md.to_catdesc() assert actual_catdesc == expected_catdesc

Spot the guy who uses base unittest :) Thanks.

… for Ultra

jtniehof · 2026-02-18T20:25:41Z

Saw this draft PR and thought I would give some feedback.

Thanks for many good catches, @subagonsouth. I think I've hit them all, but I left the conversations unresolved so you can check.

I have to update the L3 code and then I'm going to make one last check of how these display in CAVA before marking ready.

subagonsouth

Great! Your changes cleaned this up nicely.

jtniehof · 2026-02-20T14:43:10Z

Thanks! Jamey agrees that any concerns with display in CAVA are issues for CAVA, and we should proceed with this. So marking ready.

subagonsouth

Thanks for implementing this!

jtniehof added 2 commits February 17, 2026 10:28

ENA: Add to_catdesc to create CATDESC from map descriptor

32a47f9

ENA: populate principal data CATDESC from descriptor

ae75a0a

jtniehof mentioned this pull request Feb 17, 2026

Populate map variable CATDESC based on descriptor. IMAP-Science-Operations-Center/imap_L3_processing#5

Draft

ENA: Change CATDESC based on feedback from Dan

0f6de5e

* Put species before the quantity * Spell out Combined (and space out from instrument name) * Special-case ISN [species] Rate

subagonsouth reviewed Feb 18, 2026

View reviewed changes

jtniehof added 3 commits February 18, 2026 12:03

Address simple PR comments for map CATDESC

a133971

ENA: Use descriptor to find principal data variable; populate CATDESC…

bbc640a

… for Ultra

Remove redundant test for Ultra L2 rectangular map from descriptor

615c0ed

subagonsouth reviewed Feb 19, 2026

View reviewed changes

jtniehof marked this pull request as ready for review February 20, 2026 14:43

laspsandoval assigned jtniehof Feb 20, 2026

laspsandoval added the CDF Related to CDF files label Feb 20, 2026

laspsandoval added this to IMAP Feb 20, 2026

jtniehof moved this to PR Open in IMAP Feb 20, 2026

subagonsouth approved these changes Feb 20, 2026

View reviewed changes

Comments

Conversation

jtniehof commented Feb 17, 2026

Change Summary

Overview

File changes

Testing

Other notes

Uh oh!

subagonsouth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtniehof commented Feb 18, 2026

Uh oh!

subagonsouth left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtniehof commented Feb 20, 2026

Uh oh!

subagonsouth left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

subagonsouth left a comment •

edited

Loading