feat(file_browser): add filter param for regex and callable filtering#9667
Merged
kirangadhave merged 4 commits intoJun 1, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
Closes marimo-team#8399 Signed-off-by: Abhinav Tarigoppula <abhinav@Abhinavs-MacBook-Air.local>
kratos0718
force-pushed
the
feat/file-browser-filter-param
branch
from
0c837e0 to
4888d06
Compare
for more information, see https://pre-commit.ci
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
Contributor
Author
|
Hi, just following up on this PR. |
Contributor
Author
|
Hi! Just checking in — all automated checks are passing and CLA is signed. Happy to make any requested changes or incorporate feedback. Thanks for your time! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the mo.ui.file_browser() UI element with a new filter keyword argument to support additional file filtering using either a regex (string or compiled re.Pattern) or a user-provided predicate callable, while keeping directories visible for navigation.
Changes:
- Added a
filterparameter tofile_browserwith support for regex-string, compiled-pattern, and callable filtering. - Applied the filter consistently in both direct directory listings and recursive directory scanning used by
ignore_empty_dirs. - Added a focused test suite covering regex, compiled patterns, callables, directory visibility, AND-semantics with
filetypes, andignore_empty_dirsinteractions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
marimo/_plugins/ui/_impl/file_browser.py |
Adds filter parameter normalization and applies it during listing and recursive empty-dir checks. |
tests/_plugins/ui/_impl/test_file_browser.py |
Adds tests validating the new filter behavior across supported input types and interactions. |
Comment on lines
+172
to
+176
| additional filter applied to files (directories are always shown | ||
| for navigation). Accepts a regex string or compiled pattern | ||
| matched against the filename, or a callable that receives the | ||
| file's `Path` and returns `True` to include it. Applied together | ||
| with `filetypes` (both must match). Defaults to None. |
Member
Contributor
|
@kirangadhave I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 2 files
Architecture diagram
sequenceDiagram
participant User as User
participant UI as File Browser UI
participant Filter as Filter Logic
participant FileSystem as File System
Note over User,FileSystem: Filter Applied in _list_directory and _has_files_recursive
User->>UI: mo.ui.file_browser(filter=r"^train_" | compiled | callable)
UI->>Filter: Store filter as self._filter
alt filter is str
Filter->>Filter: Compile to re.Pattern
else filter is re.Pattern
Filter->>Filter: Use as-is
else filter is callable
Filter->>Filter: Use as-is
else filter is None
Filter->>Filter: Set to None (no filtering)
end
Note over UI,FileSystem: Listing a directory
UI->>UI: _list_directory(path, ...)
loop For each file in directory
alt File is a directory
UI->>FileSystem: Check directory contents
alt ignore_empty_dirs enabled
UI->>UI: _has_files_recursive(directory)
UI->>FileSystem: Check files recursively
Note over UI,FileSystem: NEW: filter applied in recursive check
alt Filter is re.Pattern
UI->>Filter: filter.search(filename)
else Filter is callable
UI->>Filter: filter(file_path)
end
alt No matching files
UI->>UI: Skip directory
else Matching files found
UI->>UI: Include directory
end
end
UI-->>UI: Always include directory for navigation
else File is not a directory
alt filetypes is set
UI->>FileSystem: Check file extension
alt Extension not in filetypes
UI->>UI: Skip file
end
end
alt filter is not None
alt Filter is re.Pattern
UI->>Filter: filter.search(filename)
else Filter is callable
UI->>Filter: filter(file_path)
end
alt No match
UI->>UI: Skip file
end
end
end
end
UI-->>User: Return filtered file list
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Extract filter evaluation into a single _passes_filter helper used by both _list_directory and _has_files_recursive, removing the duplicated regex/callable branching that could drift between the two. The helper also wraps a callable filter in try/except so one file that makes the callable raise no longer crashes the entire directory listing — it is treated as a non-match and logged. Adds a regression test for the exception-isolation behavior.
Contributor
Author
|
Thanks for the review! Addressed both findings:
Added a regression test ( |
kirangadhave
requested changes
Jun 1, 2026
Member
kirangadhave
left a comment
kirangadhave
left a comment
There was a problem hiding this comment.
Hi @kratos0718, the PR looks great. Please address the comments and we should be good to merge.
Overall note:
Please add logging with LOGGER.debug(...) to except blocks, e.g.
try:
return bool(self._filter(file))
except OSError as e:
LOGGER.debug(f"file_browser filter could not evaluate {file}: {e}")we should add logging in other places as well, but for now please add it to the new code you're adding in this PR.
…ng note Per review feedback on _passes_filter: - Catch OSError specifically (e.g. broken symlink) and treat as no-match so the rest of the directory still lists; let any other exception propagate instead of silently swallowing it. - Log skipped files with LOGGER.debug including the file and error. - Document that the regex filter uses search() (matches within the name) and that ^/$ should be used to anchor. - Add tests for both the OSError-isolated and non-OSError-propagates paths.
Contributor
Author
|
Thanks for the detailed review @kirangadhave! Addressed everything:
Ready for another look — thanks! |
kirangadhave
approved these changes
Jun 1, 2026
Member
kirangadhave
left a comment
kirangadhave
left a comment
There was a problem hiding this comment.
🚀 Thanks for addressing the comments. Ready to merge.
Bundle ReportChanges will increase total bundle size by 20.15kB (0.08%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: marimo-esmAssets Changed:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #8399
📝 Summary
Adds a
filterkeyword argument tomo.ui.file_browser()so users can filter files using regex patterns or custom functions — not just file extensions.Three ways to use it:
filter=r"^train_"filter=re.compile(r"\.csv$", re.IGNORECASE)filter=lambda p: p.stat().st_size > 1_000_000Directories are always shown for navigation. Works together with
filetypesandignore_empty_dirs.Merge Checklist