Skip to content

Chardata moretests#7101

Merged
trexfeathers merged 12 commits into
SciTools:FEATURE_chardatafrom
pp-mo:chardata_moretests
Jun 9, 2026
Merged

Chardata moretests#7101
trexfeathers merged 12 commits into
SciTools:FEATURE_chardatafrom
pp-mo:chardata_moretests

Conversation

@pp-mo

@pp-mo pp-mo commented May 26, 2026

Copy link
Copy Markdown
Member

Closes #6920
List of issues from there ...

  • READ + WRITE
    • here and here** handling of invalid "_Encoding" values
    • n-bytes <-> n_chars width translations for all supported encoding types
    • check alternate names for encodings work, e.g. "UTF8" --> "utf-8"
      • unit test in "...unit/fileformats/netcdf/test_bytecoding_datasets.py"
      • integration test in "...integration/netcdf/test_stringdata.py"
      • DONE IN e6bdb53
    • ALL IN 4e956e0 :
      • different encodings on different variables
      • scalar char variable
    • "bad" unicode content can be read OK as character arrays (non-decode loading) : AND written back
  • READ ONLY
  • WRITE ONLY
    • ALL IN 2c276a8 :
    • error when unicode data + no _Encoding
    • error/warning when encoded strings --> over-length (from n_chars -> n_bytes=dim translation)
    • error when cube/coord data has type "Sxx" other than "S1"

Plus:

  • remove "PERSIST_TESTFILES" and other debug in test_stringdata / tests_bytecoding_datasets

scitools-ci[bot]

This comment was marked as resolved.

@pp-mo pp-mo changed the base branch from main to FEATURE_chardata May 26, 2026 17:32
@pp-mo pp-mo force-pushed the chardata_moretests branch from a23c93e to 68c121e Compare May 27, 2026 09:55
@pp-mo pp-mo force-pushed the chardata_moretests branch from 68c121e to a7e928a Compare May 27, 2026 10:07
@codecov

This comment was marked as outdated.

@pp-mo pp-mo dismissed scitools-ci[bot]’s stale review May 27, 2026 15:28

Change to pre-commit config merged from main
(into feature-branch)

@pp-mo pp-mo force-pushed the chardata_moretests branch from d19908c to e6bdb53 Compare May 27, 2026 16:44
@pp-mo pp-mo force-pushed the chardata_moretests branch from a93d579 to 6988bc5 Compare May 29, 2026 11:34
@pp-mo pp-mo force-pushed the chardata_moretests branch from 6988bc5 to 2c276a8 Compare May 29, 2026 14:28
@pp-mo pp-mo marked this pull request as ready for review May 29, 2026 15:38
@pp-mo

pp-mo commented May 29, 2026

Copy link
Copy Markdown
Member Author

OK I'm finally happy with this!

In my mind, this confirms that the new chardata support facility is "safe to merge"™️
- because above all, we do want to get rid of a feature branch ASAP

Some issues remains in the original plan though.
For reference :

@pp-mo pp-mo mentioned this pull request Jun 5, 2026

@trexfeathers trexfeathers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Read through the total changes
  • Has this delivered on each checklist item?
  • Has this removed all TODO and/or commented code?

Comment thread lib/iris/fileformats/netcdf/saver.py Outdated
Comment thread lib/iris/tests/integration/netcdf/test_stringdata.py Outdated
@pp-mo pp-mo closed this Jun 9, 2026
@pp-mo

pp-mo commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

Weird problem here where GitHub isn't picking up a commit added to the fork branch.
Close+reopen to see if it fixes..

@pp-mo pp-mo reopened this Jun 9, 2026
@pp-mo

pp-mo commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

OK I think I covered your comments now?

@trexfeathers trexfeathers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @pp-mo 👍

@trexfeathers trexfeathers merged commit eaf35e9 into SciTools:FEATURE_chardata Jun 9, 2026
24 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Additional tests for new chardata handling #6898

2 participants