Skip to content

Conversation

flamingbear
Copy link
Member

@flamingbear flamingbear commented Sep 16, 2025

Description

This PR corrects the keyword args passed to open_datatree, ensuring that the concat_character behavior for xarray is enabled.

That behavior is to "concatenate along the last dimension of character arrays to form string arrays. Dimensions will only be concatenated over (and removed) if they have no corresponding variable and if they are only used as the last dimension of character arrays."

Our incorrect flags were discovered as a result of a fix to the filtering of decoders in xarray's 2025.3.0 release

Jira Issue ID

DAS-2419

Local Test Steps

Pull this branch and make sure the tests pass.

❯ ./bin/build-image && ./bin/build-test && ./bin/run-test

Run this command and verify the output is openable in panoply for plotting.

curl -Ln -bj 'http://localhost:3000/C1263085200-NSIDC_CUAT/ogc-api-coverages/1.0.0/collections/parameter_vars/coverage/rangeset?granuleId=G1263085428-NSIDC_CUAT&subset=lat(25.0%3A49.8)&subset=lon(-127.0%3A-65.0)&variable=EASE2_global_projection&variable=GEO%2Flatitude&variable=GEO%2Flongitude&variable=NEE%2Fnee_mean&variable=RH%2Frh_mean&variable=x&variable=y&serviceId=S1273752002-EEDTEST' -o spl4cmdl-localhost.nc4

PR Acceptance Checklist

  • Jira ticket acceptance criteria met.
  • CHANGELOG.md updated to include high level summary of PR changes.
  • docker/service_version.txt updated if publishing a release.
  • Tests added/updated and passing.
  • Documentation updated (if needed).

This gets around a regression in the 2025.8.0 xarray that required h5netcdf to
write NetCDF files.
decode_cf=False turns off all CF decoding by default including
concat_characters, even when it's specified.
@flamingbear flamingbear marked this pull request as draft September 16, 2025 14:54
@flamingbear flamingbear marked this pull request as ready for review September 16, 2025 15:02
decode_times=False,
decode_timedelta=False,
decode_coords=False,
decode_cf=False,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we shouldn't be using this flag if we want to do any decoding. if this is false, all the other flags are ignored and also discarded. The current set of flags is probably overkill, but it is explicit and doing what we want. concatting strings and leaving everything else alone

Copy link
Contributor

@joeyschultz joeyschultz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. I ran the unit tests and they all passed. I ran into permission issues for the collection specified in the test request, so I ran the request below for SPL4CMDL v008 instead.

http://localhost:3000/C1273193292-NSIDC_CUAT/ogc-api-coverages/1.0.0/collections/parameter_vars/coverage/rangeset?granuleId=G1273928437-NSIDC_CUAT&subset=lat(25.0%3A49.8)&subset=lon(-127.0%3A-65.0)&variable=EASE2_global_projection&variable=GEO%2Flatitude&variable=GEO%2Flongitude&variable=NEE%2Fnee_mean&variable=RH%2Frh_mean&variable=x&variable=y&serviceId=S1273752002-EEDTEST

I was able to open the output in panoply and plot the variables.

Copy link
Member

@owenlittlejohns owenlittlejohns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great sleuthing the other day to get to the bottom of this!

@flamingbear flamingbear merged commit 1e27034 into main Sep 16, 2025
4 checks passed
@flamingbear flamingbear deleted the mhs/DAS-2419/update-xarray-use-correct-params branch September 16, 2025 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants