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

use _repr_inline_ for indexes that define it #7183

Merged
merged 17 commits into from
Oct 19, 2022

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Oct 18, 2022

Also, some tests for the index summarizer.

@keewis keewis changed the title use _repr_inline for indexes that define it use _repr_inline_ for indexes that define it Oct 18, 2022
@benbovy
Copy link
Member

benbovy commented Oct 18, 2022

Great @keewis!

One question: should we let repr_inline display the class name or should we reserve a column for this and use repr_inline for other things? I.e., like variables have a dtype column and another column for values preview or other inline info.

@keewis
Copy link
Collaborator Author

keewis commented Oct 18, 2022

Not sure. As far as I can tell, the information that could be put in the repr would be:

  • the names of the wrapped variables
  • the index class name
  • any parameters passed to the constructor
  • parameters inferred from the data?

I guess the separation very much depends on how we want to display the indexes. For now, I'd imagine something like this would be great:

[lat, lon]    KDTree(ranges={"lat": [-45, 45], "lon": [-60, 30]}, leaf_size=10)

where leaf_size was passed to the constructor and ranges is computed from the mins / maxes attributes of the k-d tree.

@benbovy
Copy link
Member

benbovy commented Oct 18, 2022

Yeah I think we could let the whole line after the 1st column (coordinate names) be customized by the index.

@keewis keewis mentioned this pull request Oct 18, 2022
3 tasks
@benbovy
Copy link
Member

benbovy commented Oct 19, 2022

Looks all good to me!

Do you want to add a what's new entry here or add it in #7185 with a link to this PR?

@keewis
Copy link
Collaborator Author

keewis commented Oct 19, 2022

I will add that in #7185 (#6795 is also missing an entry)

@keewis keewis added the plan to merge Final call for comments label Oct 19, 2022
@keewis
Copy link
Collaborator Author

keewis commented Oct 19, 2022

okay, let's merge this and update #7183.

@keewis keewis merged commit 7379923 into pydata:main Oct 19, 2022
@keewis keewis deleted the index-repr_inline branch October 19, 2022 14:06
@benbovy benbovy mentioned this pull request Oct 25, 2022
49 tasks
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.

2 participants