feat(ui.file_browser): add flexible path_filter support#8607
feat(ui.file_browser): add flexible path_filter support#8607tsubasakong wants to merge 14 commits intomarimo-team:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
for more information, see https://pre-commit.ci
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
There was a problem hiding this comment.
Pull request overview
Adds flexible path_filter support to mo.ui.file_browser, enabling additional filtering via glob patterns or callables while preserving existing filetypes behavior and adding tests to validate the new functionality.
Changes:
- Added optional
path_filterparameter (glob string orCallable[[Path], bool]) and applied it during directory listing. - Combined
filetypesfiltering withpath_filterusing AND logic, and appliedpath_filterto directories inselection_mode="directory". - Added tests for glob filtering, callable filtering, directory-mode filtering, and invalid filter validation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| marimo/_plugins/ui/_impl/file_browser.py | Introduces path_filter type/validation and integrates it into listing + recursive empty-dir checks. |
| tests/_plugins/ui/_impl/test_file_browser.py | Adds coverage for glob/callable filters, directory-mode behavior, and invalid path_filter inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not self._matches_file_filters(item): | ||
| continue | ||
| return True |
There was a problem hiding this comment.
_has_files_recursive now applies path_filter (via _matches_file_filters) in addition to filetypes when determining whether a directory is “empty”. The surrounding documentation/comments currently describe only filetype-based filtering (e.g., in the ignore_empty_dirs doc text). Please update the relevant docstrings/comments to reflect that both filetypes and path_filter are applied during recursive emptiness checks, so the behavior is not surprising to users.
| return bool(self._path_filter(path)) | ||
|
|
There was a problem hiding this comment.
If a user-provided callable path_filter raises (e.g., due to transient filesystem errors like stat() failing), the exception will bubble up and likely produce a low-context error originating from inside the filter. Consider wrapping callable execution in a try/except and re-raising with a higher-context message that includes the path being evaluated and indicates the failure came from path_filter (optionally chaining the original exception). This makes runtime failures significantly easier to diagnose.
| return bool(self._path_filter(path)) | |
| try: | |
| result = self._path_filter(path) | |
| except Exception as exc: | |
| raise RuntimeError( | |
| f"path_filter callable raised an exception for path {path!r}" | |
| ) from exc | |
| return bool(result) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
marimo/_plugins/ui/_impl/file_browser.py:140
- The new
path_filteris described as an additional filter, butignore_empty_dirsuses_has_files_recursive()which now applies_matches_file_filters()(and thereforepath_filter) when deciding whether a directory is “empty”. Either document this interaction here (so users understand thatignore_empty_dirs=Trueis based on the filtered view), or adjust the recursive emptiness check if the intent is to only considerfiletypes.
path_filter (Union[str, Callable[[Path], bool]], optional): Additional
path-based filtering to apply. In file mode, this is applied to files;
in directory mode, this is applied to directories. If a string is
provided, it is treated as a glob pattern (for example, "data_*.csv").
If a callable is provided, it should return True for paths that should
be included. Defaults to None.
selection_mode (Literal["file", "directory"], optional): Either "file" or "directory". Defaults to
"file".
multiple (bool, optional): If True, allow the user to select multiple
files. Defaults to True.
restrict_navigation (bool, optional): If True, prevent the user from
navigating any level above the given path. Defaults to False.
ignore_empty_dirs (bool, optional): If True, hide directories that contain
no files (recursively). Directories are scanned up to 100 levels deep
to prevent stack overflow from deeply nested structures. Directory
symlinks are skipped during traversal to prevent infinite loops.
Filetype filtering is applied recursively and is case-insensitive.
This may impact performance for large directory trees. Defaults to False.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| path_filter: Optional[PathFilter] = None, | ||
| selection_mode: Literal["file", "directory"] = "file", | ||
| multiple: bool = True, | ||
| restrict_navigation: bool = False, |
📝 Summary
Closes #8399
path_filterparameter tomo.ui.file_browser"data_*.csv") and callable filters (Callable[[Path], bool])filetypesbehavior and combine it withpath_filterusing AND logicpath_filterto directories inselection_mode="directory"🔍 Description of Changes
path_filterparameter tomo.ui.file_browser"data_*.csv") and callable filters (Callable[[Path], bool])filetypesbehavior and combine it withpath_filterusing AND logicpath_filterto directories inselection_mode="directory"Validation:
python3 -m compileall marimo/_plugins/ui/_impl/file_browser.py tests/_plugins/ui/_impl/test_file_browser.pyCloses #8399
📋 Checklist