Check minknow experiment folder on startup - #86
Open
berndbohmeier wants to merge 2 commits into
Open
berndbohmeier wants to merge 2 commits into
berndbohmeier wants to merge 2 commits into
Conversation
We checked that the minknow experiment folder is created only a few sec after the minknow experiment is started, probably after some checks are run. We decided this is good enough to check it so that people who misspelled something will be notified. Some folders inside are only created much later, so we should not wait for that.
berndbohmeier
requested review from
danieljbridges
and
a lite review from Copilot
September 11, 2026 14:14
There was a problem hiding this comment.
🟡 Changes recommended
Four moderate findings remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds startup validation for MinKNOW experiment folders and improved typo suggestions for experiment and metadata names.
Changes:
- Validates experiment directories during path resolution.
- Adds close-match suggestions for experiments and metadata files.
- Routes input errors through user-facing CLI messages.
File summaries
| File | Summary and findings |
|---|---|
src/nomadic/util/minknow.py |
Adds experiment validation and similarity matching. Moderate findings: use is_dir(), exclude reserved directories, and normalize case-insensitive comparisons (3, 3, and 1 votes). |
src/nomadic/realtime/commands.py |
Adds metadata matching and error handling. Moderate finding: restrict candidates to .csv and .xlsx files (2 votes). |
Review details
Suppressed comments (1)
src/nomadic/util/minknow.py:140
- The query is lowercased but the candidate directory names are not, so the supposed close-match lookup is still case-sensitive. For an uppercase MinKNOW experiment such as
ABCDEF, even a one-character typo can fall through to the generic message; normalize both sides for comparison while returning the original directory name.
close_matches = difflib.get_close_matches(
experiment_name.lower(),
[d.name for d in minknow_path.glob("*") if d.is_dir()],
cutoff=0.8,
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
berndbohmeier
force-pushed
the
feature/check-minknow-folder-directly
branch
from
September 11, 2026 15:24
3b97e10 to
60a077d
Compare
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.
We checked that the minknow experiment folder is created only a few sec after the minknow experiment is started, probably after some checks are run. We decided this is good enough to check it so that people who misspelled something will be notified. Some folders inside are only created much later, so we should not wait for that.