-
Notifications
You must be signed in to change notification settings - Fork 3
Add itables & .interactive
attribute
#80
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
Conversation
…of strings silently caused later issues with search
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces support for interactive table displays via itables while ensuring that columns_with_iterables can accept both string and list inputs in the catalog and search functionalities. Key changes include:
- Extending tests to cover a case where "columns_with_iterables" is provided as a string.
- Updating type annotations for parameters in the constructor and search functions.
- Adding an interactive property that leverages itables for displaying the catalog.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
File | Description |
---|---|
tests/test_core.py | Expanded test cases to validate both string and list inputs for kwargs. |
src/intake_dataframe_catalog/core.py | Updated constructor type hints and added an interactive property to display the catalog. |
src/intake_dataframe_catalog/_search.py | Refined type annotation for columns_with_iterables and added conversion for string inputs. |
pyproject.toml; ci/environment-*.yml | Added itables as a dependency for both development and production environments. |
Comments suppressed due to low confidence (1)
src/intake_dataframe_catalog/core.py:598
- The 'interactive' property returns the result of itables.show(self._df), which may produce side effects or unexpected results upon simple property access. Consider converting this property into a method (e.g., 'show_interactive') or update the return type annotation to clearly indicate its intent.
def interactive(self) -> None:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really neat. To make it easily useable, I think we want to:
- remove the
yaml_column
column from the interactive table - have the
name_column
column as the first column
Hopefully this isn't too hard?
I think removing the yaml column is straightforward - trying to group by name might get a bit messier.
I think this might be a quirk of the column types, but good catch. I'll dig into it properly. |
I don't even mean grouping by name. I just mean having it as the first column. Not critical though |
…ting out with itables
…ld stop us blowing our dataframe up too large (and wrongly, anyway) if we have multiple iterable columns
Updated this to check all iterable columns and 'jointly explode' them if they all contain lists which have the same length lists in each record - functionality contained in the Kind of hard to explain what this means with words, but the tests should hopefully make the intent clear. |
Looks really great @charles-turner-1 . However, I found the widget unhelpful, mostly because of how much it truncated things. This was in browser jupyterlab: ![]() Could we just remove for now? |
Without the widget, there's no way to search on multiple fields at once - I haven't given much thought as to whether this is something that users are going to want? Happy to remove it if it's probably not something users will want - otherwise I might see if the package exposes a way to fiddle with the CSS and fix the truncation. |
Good point. That probably is something that users would fine useful.... |
Cool - I'll dig into the CSS, I think we should be able to force the widgets to be wider. |
Could you give me a snippet of code that reproduces those truncated widgets @dougiesquire, as well as the resolution/orientation of the screen you're opening it on? I've opened up an interactive catalog in an online Jupyter instance and it looks fine on my laptop screen (see below, 13 inch Macbook, 'more space' setting). This frontend-y stuff can be a bit of a nightmare to debug/reproduce/test. ![]() |
I was just running the example in the |
Cool, I'll have a poke at the quickstart and see if I can track down any issues. Don't have a monitor to hand, but when I was working on this at home I didn't see any issues. Maybe @marc-white could try if he's in the office? |
I was using Firefox. And the output is still truncated in a private session without any extensions active... But, things also look good to me with Safari, so shall we just add a note as you suggest and move on? |
Interesting. Looks like firefox uses a different browser engine (not Webkit or Chromium), so must be something related to that. I'll add a note here, and also on the intake catalog docs. |
…l issues relating to different browsers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look great - thanks @charles-turner-1. I think users are going to find this very useful
See also https://github.com/intake/intake-esm/pull/723/files.
Added a
.interactive
property, aiming to keep everything as consistent as possible.Also fixed a bug where passing
columns_with_iterables
as a string wouldn't break anything at load time but would fail when we start searching.