-
Notifications
You must be signed in to change notification settings - Fork 192
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
feat: datasets panel #1427
feat: datasets panel #1427
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
36151ce
to
0a83dbb
Compare
@@ -39,6 +39,13 @@ export const Sidebar: React.FC = () => { | |||
> | |||
{renderIcon("dependencies")} | |||
</SidebarItem> | |||
<SidebarItem | |||
tooltip="Data sources" |
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.
tooltip="Data sources" | |
tooltip="Explore data sources" |
import { autoInstantiateAtom } from "@/core/config/config"; | ||
import { DataColumnPreview } from "@/core/kernel/messages"; | ||
|
||
export const DataSourcesPanel: React.FC = () => { |
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.
Could it be possible to suggest or even prompt users to install altair
if it's not installed, to get plots?
Doesn't have to be in this PR, but could be a good follow-up to make the plots feature more discoverable.
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.
easy to add with how we send back errors
size="icon" | ||
onClick={Events.stopPropagation(() => onAddTable(table))} | ||
> | ||
<PlusSquareIcon className="h-3 w-3" /> |
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.
Add tooltip? Similar to what's there for the add chart button.
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.
done
@abc.abstractmethod | ||
def altair(self, data: Any, column: str) -> Any: | ||
raise NotImplementedError |
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.
If you wanted to minimize code duplication, I think this method could be:
glbls = {"data": data, "column": column, "alt": alt}
exec(textwrap.dedent(self.altair_code(data, column)), glbls)
# assumes that `altair_code` defines a `_chart` variable
return glbls["_chart"]
Probably simpler to keep your code as is for now, since you also get type and syntax checking the way you've written it. But thought I'd mention just for fun
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.
ah very cool. i think for readability (and possibly they could diverge in the future), ill leave for now
Co-authored-by: Akshay Agrawal <akshay@marimo.io>
📝 Summary
This adds a new panel that intelligently discovers datasets (right now pandas, polars, pyarrow). and will show its shape and schema. From here you can explore its columns and get quick charts based on them - which you can add directly to your notebook.
The goal is to extend this to external datasources and other types (DBs, etc)
🔍 Description of Changes