Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Strings with a single char in Python #1585

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

franzpoeschel
Copy link
Contributor

Follow-up to #1517

Problem: In pipe/__main__.py, dest.set_attribute(key, attr, attr_type) cannot distinguish strings with size 1 from chars (except for explicit char types signed char and unsigned char).

Before #1517, that call converted chars up to string, now it converts strings down to chars.

Old behavior:

  string    /data/600/fields/i_all_energyDensity/axisLabels                         attr   = {"z", "y", "x"}                                                                                                                                                                                                               

New behavior

  char      /data/600/fields/e_all_energyDensity/axisLabels                         attr   = {z, y, x}

Since values such as axisLabels = ["x", "y", "z"] are very frequent, the old behavior was less intrusive.

This PR:

  1. Switches back to the old behavior (i.e. cancels out the attempts to cast to char, but keeps the attempts to cast to explicit signed or unsigned char)
  2. Makes Attribute::getOptional<>() more flexible in such conversions (Currently openpmd-pipe creates unreadable files)

Instead of "char in doubt"
@franzpoeschel franzpoeschel added this to the 0.15.3 milestone Jan 8, 2024
@franzpoeschel franzpoeschel requested a review from ax3l January 17, 2024 11:29
@franzpoeschel franzpoeschel merged commit 3a8a220 into openPMD:dev Jan 17, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant