Skip to content

Conversation

@bcbnz
Copy link
Contributor

@bcbnz bcbnz commented Apr 13, 2021

The standard repr() already handled non-string attribute names, but the HTML formatter failed when trying to escape HTML entitites in non-string names. This just calls str() before escape(). It also includes tests for Dataset, DataArray and Variable.

Reported in #5146. Note that there may be a need to do the same for dimension names if they are allowed to be strings. Currently dimensions must be created as strings but can later be renamed to non-strings, see #5148. Dimensions can be non-str, updated.

  • Tests added
  • Passes pre-commit run --all-files

The standard repr() already handled non-string attribute names, but the
HTML formatter failed when trying to escape HTML entitites in non-string
names. This just calls str() before escape(). It also includes tests for
Dataset, DataArray and Variable.

Related issue is #5146.
Not currently working for datasets as dimension sorting does not yet
handle non-str keys.
@bcbnz bcbnz changed the title Convert attribute keys to strings when generating HTML repr Convert attribute and dimension names to strings when generating HTML repr Apr 13, 2021
@bcbnz
Copy link
Contributor Author

bcbnz commented Apr 13, 2021

Added conversion of dimension names since non-string dimensions are intended to be supported. This will still fail for datasets as the dimension sorting does not yet support this.

@keewis
Copy link
Collaborator

keewis commented Apr 29, 2021

thanks, @bcbnz. You could add a entry to whats-new.rst (bug fixes), but otherwise this should be ready.

@max-sixty
Copy link
Collaborator

Looks like other people are hitting this given #5240.

Would be great to get this in for 0.18.0 #5232

@shoyer
Copy link
Member

shoyer commented May 2, 2021

whats-new.rst is really optional (nice to have)... I would vote for just merging at this point! I will probably do so tomorrow :)

@keewis keewis mentioned this pull request May 2, 2021
13 tasks
@shoyer shoyer merged commit 1c198a1 into pydata:master May 4, 2021
@shoyer
Copy link
Member

shoyer commented May 4, 2021

Thanks @bcbnz !

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.

5 participants