Skip to content

Conversation

@BenjaminRuston
Copy link
Collaborator

@BenjaminRuston BenjaminRuston commented Oct 21, 2025

Description

the nstring dimension is non-standard and is removed

Issue(s) addressed

Resolves #1719

Impact

IODA file without dimension nstring

Checklist

  • I have performed a self-review of my own code
  • I have run the unit tests before creating the PR

@BenjaminRuston BenjaminRuston marked this pull request as ready for review October 23, 2025 14:41
@BenjaminRuston
Copy link
Collaborator Author

@ibanos90 do you know why the nstring dimension is included? Do not believe we use it, and causes issues if added for radiance data. Similarly the attributes max_datetime and min_datetime we don't use and will remove this can be determined from the MetaData/dateTime themselves

@amstokely if you remember anything please let us know but may remove the nstring dimension and attributes max_datetime and min_datetime

@ibanos90
Copy link
Contributor

@ibanos90 do you know why the nstring dimension is included? Do not believe we use it, and causes issues if added for radiance data. Similarly the attributes max_datetime and min_datetime we don't use and will remove this can be determined from the MetaData/dateTime themselves

@amstokely if you remember anything please let us know but may remove the nstring dimension and attributes max_datetime and min_datetime

Hi @BenjaminRuston, I believe the nstring dimension and max_datetime and min_datetime variables may be inherited from older IODA versions and kept in the files when we used the IODA upgraders, but we do not use in any of our filters. I believe they can be safely removed.

@amstokely
Copy link
Contributor

@BenjaminRuston @ibanos90 I believe this is a legacy attribute from when fixed-length character arrays were used for strings. Since we now use the NetCDF string type, nstring is no longer needed for data representation.

That said, much of the legacy obs2ioda code relies on the number of attributes, variables, and dimensions for indexing (unfortunately). Removing nstring would reduce the dimension count and could break parts of the code that depend on it. I ran into a similar issue before when removing unused variables caused indexing errors.

I'm not sure what level of testing ioda-converters currently has for obs2ioda, but the test suite I wrote back when NCAR maintained it caught these issues quickly. It may also affect analysis scripts that use index-based loops dependent on the number of dimensions.

@BenjaminRuston
Copy link
Collaborator Author

@amstokely my preference is to try to keep the attribute in memory where possible and just not write to the final file

I spoke with @ibanos90 and we believe it can be removed from the output, and we'll try to monitor the output in our applications. My preference is to move forward with this shiny new version and make changes as needed.

From everything I've seen so far it is safe to remove nstring from the files written, will also remove the min/max dateTime attributes

Copy link
Collaborator

@fcvdb fcvdb left a comment

Choose a reason for hiding this comment

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

Thanks Ben!

@BenjaminRuston
Copy link
Collaborator Author

@ibanos90 think this is ready, would be good to have confirmation on your side that it is a lateral move

@amstokely you were right there is something buried in there somewhere appears to be conventional data related so only skipping the nstring dimension for non-conventional data types

Copy link
Contributor

@fabiolrdiniz fabiolrdiniz left a comment

Choose a reason for hiding this comment

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

Thanks, @BenjaminRuston!

@fabiolrdiniz fabiolrdiniz added the OBS OBS processing, UFO label Nov 10, 2025
oid sha256:34ba505d99286afae2d3e073cd1c98a3abed83cfdc36ac5dab2f61d57165c6b3
size 129269
oid sha256:f4f9d437da9fa0597e3fcef1890cf168d6d2b6782a3e2b8548c7f47476948b39
size 203243
Copy link
Contributor

Choose a reason for hiding this comment

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

This file size seem to be bigger than original.

@BenjaminRuston BenjaminRuston merged commit e220318 into develop Nov 10, 2025
3 checks passed
@BenjaminRuston BenjaminRuston deleted the feature/ahi_remove_nstring branch November 10, 2025 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OBS OBS processing, UFO

Projects

None yet

Development

Successfully merging this pull request may close these issues.

obs2ioda writing extra dimension and attributes

7 participants