Skip to content

Convert attribute and dimension names to strings when generating HTML repr #5149

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

Merged
merged 3 commits into from
May 4, 2021
Merged

Convert attribute and dimension names to strings when generating HTML repr #5149

merged 3 commits into from
May 4, 2021

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