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

join together duplicate entries in the text repr #7225

Merged
merged 27 commits into from
Jul 20, 2023

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Oct 26, 2022

Indexes contains one entry per coordinate, even if there are indexes that index multiple coordinates. This deduplicates the entries, but the format is not quite clear.

The formatting options we were able to come up with:

  1. separate with just newlines (see e29aeb9):
Indexes:
    one    CustomIndex
    two
    three  PandasIndex
  1. mark unique indexes with a prefix to make it look like a list (see 9b90f8b):
Indexes:
  - one    CustomIndex
    two
  - three  PandasIndex
  1. use unicode box components (in front of the coordinate names) (see 2cec070):
Indexes:
  ┌ one    CustomIndex
  │ two
  └ three
    four  PandasIndex
  1. use unicode box components (after the coordinate names) (see 492ab47):
Indexes:
    one   ┐ CustomIndex
    two   │
    three ┘
    four    PandasIndex

For the unicode box components, we can choose between the light and heavy variants.

@benbovy and I think the unicode variants (especially variant 3) are the easiest to understand, but we would need to decide whether we care about terminals that don't support unicode.

Edit: in the meeting we decided that support for the subsection of unicode should be common enough that we can use it. I'll clean this PR up to implement option 3, then.

@keewis keewis force-pushed the indexes-repr-join-text branch from 7af0b29 to 492ab47 Compare October 26, 2022 13:20
Support for the box components should be common enough that we can use
it (and the worst that can happen is that those characters are
displayed with a replacement glyph – usually a diamond with a question
mark)
xarray/core/formatting.py Outdated Show resolved Hide resolved
xarray/core/formatting.py Outdated Show resolved Hide resolved
xarray/core/formatting.py Outdated Show resolved Hide resolved
xarray/core/formatting.py Outdated Show resolved Hide resolved
keewis and others added 2 commits October 26, 2022 20:57
Copy link
Collaborator

@headtr1ck headtr1ck left a comment

Choose a reason for hiding this comment

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

Some tests and this looks good.

doc/whats-new.rst Outdated Show resolved Hide resolved
xarray/core/formatting.py Outdated Show resolved Hide resolved
doc/whats-new.rst Outdated Show resolved Hide resolved
@headtr1ck headtr1ck added plan to merge Final call for comments and removed needs work labels Jul 13, 2023
@keewis
Copy link
Collaborator Author

keewis commented Jul 13, 2023

@headtr1ck, any ideas how to resolve the mypy errors related to the monkeypatching in the test?

@keewis
Copy link
Collaborator Author

keewis commented Jul 16, 2023

the typing (especially the typing ignores of the monkeypatching in the test) could use another review, but other than that this should be ready for merging.

@keewis
Copy link
Collaborator Author

keewis commented Jul 18, 2023

there appears to be something wrong with the windows CI (all python versions): the "version info" step, which usually takes about 10 seconds, has been running for more than 4 hours now (and this is not just this PR, #7999 is affected, as well)

@dcherian dcherian enabled auto-merge (squash) July 19, 2023 18:27
@dcherian
Copy link
Contributor

Thanks @keewis

@dcherian dcherian disabled auto-merge July 20, 2023 21:13
@dcherian dcherian merged commit 6b1ff6d into pydata:main Jul 20, 2023
@keewis keewis deleted the indexes-repr-join-text branch July 24, 2023 18:37
dcherian added a commit to dcherian/xarray that referenced this pull request Jul 24, 2023
…lazy-array

* upstream/main: (153 commits)
  Add HDF5 Section to read/write docs page (pydata#8012)
  [pre-commit.ci] pre-commit autoupdate (pydata#8014)
  Update interpolate_na in dataset.py (pydata#7974)
  improved docstring of to_netcdf (issue pydata#7127) (pydata#7947)
  Expose "Coordinates" as part of Xarray's public API (pydata#7368)
  Core team member guide (pydata#7999)
  join together duplicate entries in the text `repr` (pydata#7225)
  Update copyright year in README (pydata#8007)
  Allow opening datasets with nD dimenson coordinate variables. (pydata#7989)
  Move whats-new entry
  [pre-commit.ci] pre-commit autoupdate (pydata#7997)
  Add documentation on custom indexes (pydata#6975)
  Use variable name in all exceptions raised in `as_variable` (pydata#7995)
  Bump pypa/gh-action-pypi-publish from 1.8.7 to 1.8.8 (pydata#7994)
  New whatsnew section
  Remove future release notes before this release
  Update whats-new.rst for new release (pydata#7993)
  Remove hue_style from plot1d docstring (pydata#7925)
  Add new what's new section (pydata#7986)
  Release summary for v2023.07.0 (pydata#7979)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plan to merge Final call for comments topic-indexing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants