Harden path-handling after pathlib migration - #3421
Merged
Merged
Conversation
deruyter92
force-pushed
the
jaap/pathlib-hardening
branch
from
July 20, 2026 15:58
7be1695 to
16c9c18
Compare
MMathisLab
self-requested a review
July 21, 2026 02:13
MMathisLab
approved these changes
Jul 21, 2026
The old code checks only the first row (df.index[0]) to decide whether to split on / or \, then applies that one separator to the entire DataFrame. The new approach normalizes all backslashes to forward slashes first, then splits. So mixed-separator DataFrames (e.g. cross platform merges) work correctly.
…athlib `Path` - Deprecate `grab_files_in_folder` (replaced by `collect_video_paths`) - Replace all internal callers with `collect_video_paths` - Migrate `check_if_not_analyzed`, `save_data`, `_convert_h5_files_to`, and `find_analyzed_data` from `str`-based path ops to pathlib (.stem, .with_suffix, .with_name, .removeprefix) - Remove redundant `Path()` wraps and `str()` conversions at call sites
deruyter92
force-pushed
the
jaap/pathlib-hardening
branch
from
July 21, 2026 06:48
92cdc1a to
0a9bbbc
Compare
deruyter92
force-pushed
the
jaap/pathlib-hardening
branch
from
July 21, 2026 10:15
5f81296 to
ece7c7d
Compare
deruyter92
force-pushed
the
jaap/pathlib-hardening
branch
from
July 21, 2026 10:51
ece7c7d to
849644a
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.
Summary
Follow-up of #3350 (migration to pathlib Path), after auditing for remaining fragile patterns. This PR hardens the codebase against
Path/strtype confusion. Catches fragile patterns that work today but could silently break if types shift.Changes
Path + strTypeError —project_path + "_bak"at a live callsite (was in dead code, but now safe regardless).
cfg["init_weights"]usage — the two places that concatenatedstrings onto it now use
Path.with_suffix()instead._robust_path_split(trainingsetmanipulation) androbust_split_path(conversioncode) — both were pre-pathlib relics thatsplit strings on
/or\only to immediately re-join viaPath(*). Allcallers now use
Path().stemorPath()directly.guarantee_multiindex_rows— dropped the fragilesingle-row heuristic (
"/" in path[0]) for a consistentnormalize-then-split approach.
grab_files_in_folder— replaced all internal callers withcollect_video_paths, added@deprecateddecorator.save_data,check_if_not_analyzed,_convert_h5_files_tofrom string-path operations to pathlib (
.stem,.with_suffix,.with_name,.removeprefix).Path()wraps where variables were alreadyPath,and removed
str()conversions wherePathis accepted natively.