feat: use variable name as download filename in dataframe viewer#8227
Conversation
The rich dataframe viewer now names download files after the variable (e.g., "my_df.csv" instead of "download.csv"). The backend already extracts and sends the variable name; this change threads it through the frontend components to the download action. Closes marimo-team#8095
for more information, see https://pre-commit.ci
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add downloadFileName to the withData Zod schema so it isn't stripped during parsing - Sanitize the filename: trim whitespace, strip existing extension to prevent double extensions, fall back to "download" if empty
for more information, see https://pre-commit.ci
Infer the variable name via inspect.currentframe() (same pattern as DataFramePlugin) and pass it as download-file-name in component args so the frontend can use it for the download filename.
|
Good catch — I've now added the backend side. The table's |
There was a problem hiding this comment.
Pull request overview
Threads a dataframe/table variable name through the plugin/data-table stack so that downloads from the rich dataframe viewer (and tables) use a meaningful filename instead of a generic download.*.
Changes:
- Added
dataframeNamesupport to the dataframe viewer plugin and passed it into the shared data-table download action. - Added
downloadFileNamesupport to the table plugin and propagated it through the data-table components. - Updated download action to derive
${baseName}.${ext}and added a Python unit test for table filename propagation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/_plugins/ui/_impl/test_table.py | Adds a unit test asserting ui.table() sets the download filename argument. |
| marimo/_plugins/ui/_impl/table.py | Infers a download filename via frame locals and emits it as a component arg. |
| frontend/src/plugins/impl/data-frames/DataFramePlugin.tsx | Accepts dataframeName and passes it to the shared data-table as downloadFileName. |
| frontend/src/plugins/impl/DataTablePlugin.tsx | Adds downloadFileName to plugin schema/props and passes it into the table component tree. |
| frontend/src/components/data-table/download-actions.tsx | Uses downloadFileName to name downloaded files instead of download.*. |
| frontend/src/components/data-table/data-table.tsx | Plumbs downloadFileName into TableActions. |
| frontend/src/components/data-table/TableActions.tsx | Passes downloadFileName into DownloadAs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
marimo/_plugins/ui/_impl/table.py
Outdated
| if frame is not None and frame.f_back is not None: | ||
| for var_name, var_value in frame.f_back.f_locals.items(): | ||
| if var_value is data: | ||
| download_file_name = var_name |
There was a problem hiding this comment.
The variable-name inference compares locals against data after add_selection_column() may have converted/replaced the original input (notably for dataframe inputs). For ui.table(df) this will likely fail to match the caller’s df variable (identity check won’t match the new object), so downloads will still fall back to "download". Consider capturing the original argument before mutation (or compute the name before add_selection_column()), and use that for the identity match / filename propagation.
marimo/_plugins/ui/_impl/table.py
Outdated
| try: | ||
| frame = inspect.currentframe() | ||
| if frame is not None and frame.f_back is not None: | ||
| for var_name, var_value in frame.f_back.f_locals.items(): | ||
| if var_value is data: | ||
| download_file_name = var_name | ||
| break | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
inspect.currentframe() can create reference cycles; without explicitly clearing the frame reference, this can delay GC in long-running sessions. Consider using a finally block to del frame (and any other frame refs) after the lookup to avoid leaking frames.
| try: | |
| frame = inspect.currentframe() | |
| if frame is not None and frame.f_back is not None: | |
| for var_name, var_value in frame.f_back.f_locals.items(): | |
| if var_value is data: | |
| download_file_name = var_name | |
| break | |
| except Exception: | |
| pass | |
| frame = None | |
| frame_back = None | |
| try: | |
| frame = inspect.currentframe() | |
| if frame is not None: | |
| frame_back = frame.f_back | |
| if frame_back is not None: | |
| for var_name, var_value in frame_back.f_locals.items(): | |
| if var_value is data: | |
| download_file_name = var_name | |
| break | |
| except Exception: | |
| pass | |
| finally: | |
| del frame | |
| del frame_back |
| const rawName = (props.downloadFileName ?? "").trim(); | ||
| const baseName = rawName.replace(/\.[^/.]+$/, "") || "download"; | ||
| downloadByURL(downloadUrl, `${baseName}.${ext}`); |
There was a problem hiding this comment.
This duplicates filename-extension stripping logic that already exists in frontend/src/utils/filenames.ts (withoutExtension/replace). Using the shared utility here would reduce duplication and keep filename rules consistent across download entry points.
| onSelect={async () => { | ||
| const downloadUrl = await getDownloadUrl(option.format); | ||
| const ext = option.format; | ||
| downloadByURL(downloadUrl, `download.${ext}`); | ||
| const rawName = (props.downloadFileName ?? "").trim(); | ||
| const baseName = rawName.replace(/\.[^/.]+$/, "") || "download"; | ||
| downloadByURL(downloadUrl, `${baseName}.${ext}`); |
There was a problem hiding this comment.
The new filename propagation logic isn’t covered by a frontend test. Since the repo already has Vitest coverage for the data-table plugin, consider adding a unit test that asserts DownloadAs calls downloadByURL with the expected ${baseName}.${ext} when downloadFileName is provided (and falls back to download.* when it isn’t).
marimo/_plugins/ui/_impl/table.py
Outdated
| download_file_name = var_name | ||
| break | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| pass | |
| LOGGER.debug( | |
| "Failed to infer download file name from caller frame.", | |
| exc_info=True, | |
| ) |
- move variable name inference before add_selection_column() so the identity check still matches the caller's original variable - clean up inspect.currentframe() refs in a finally block to avoid reference cycles in long-running sessions - log the exception instead of bare pass - use shared Filenames.withoutExtension() instead of inline regex
|
pushed fixes for the review feedback:
|
marimo/_plugins/ui/_impl/table.py
Outdated
| download_file_name = "download" | ||
| frame = None | ||
| frame_back = None | ||
| try: | ||
| frame = inspect.currentframe() | ||
| if frame is not None: | ||
| frame_back = frame.f_back | ||
| if frame_back is not None: | ||
| for var_name, var_value in frame_back.f_locals.items(): | ||
| if var_value is data: | ||
| download_file_name = var_name | ||
| break | ||
| except Exception: | ||
| LOGGER.debug( | ||
| "Failed to infer download file name from caller frame.", | ||
| exc_info=True, | ||
| ) | ||
| finally: | ||
| del frame | ||
| del frame_back |
There was a problem hiding this comment.
i would maybe refator this to a utility function, since dataframe.py uses it too
There was a problem hiding this comment.
good call, extracted it into marimo/_utils/variable_name.py — both table.py and dataframe.py now use infer_variable_name(value, fallback) instead of duplicating the frame-walking logic.
Move the `inspect.currentframe()` variable-name lookup used by both `table.py` and `dataframe.py` into `marimo/_utils/variable_name.py`. Both callers now use `infer_variable_name(value, fallback)` instead of duplicating the frame-walking logic.
Light2Dark
left a comment
There was a problem hiding this comment.
lgtm, just a small change for correctness
marimo/_utils/variable_name.py
Outdated
| from typing import Any | ||
|
|
||
| LOGGER = logging.getLogger(__name__) |
There was a problem hiding this comment.
I don't know why I can't suggest code anymore, sigh GitHub UI
from marimo import _loggers
LOGGER = _loggers.marimo_logger()Removed unused logging import.
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.19.10-dev38 |
|
Thanks @mscolnick and @Light2Dark ! Appreciate the help and feedback :) |
Summary
Downloaded files from the dataframe viewer used a generic
download.csvname. Now threads the variable name from the plugin through to the download action so the file is named after the variable (e.g.my_dataframe.csv).Closes #8095
Test Plan