Repository navigation
formatting and simplification - #2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves code quality through formatting standardization, refactoring, and bug fixes across Python scripts and Jupyter notebooks.
Changes:
- Fixed critical bug in CutMix/MixUp augmentation where CutMix incorrectly used
mixup_alphainstead ofcutmix_alpha - Refactored
trainClassifier.pyby wrapping execution code in amain()function with improved comments and configuration organization - Extracted helper functions in
tools/synth.py(prefixed with_) for better code organization - Standardized naming conventions (e.g.,
cache_path→DEFAULT_CACHE_PATH) and formatting across all files - Improved documentation and error messages throughout the codebase
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| trainClassifier.py | Refactored into main() function with improved comments, reorganized configuration, but introduced hardcoded class count assertions |
| tools/transforms.py | Fixed critical bug where CutMix used mixup_alpha instead of cutmix_alpha; simplified getCollateFn; improved documentation |
| tools/synth.py | Extracted helper functions (_slerp, _get_middle_elements, _remove_middle); improved documentation and error messages |
| tools/data.py | Renamed constants to uppercase (DEFAULT_CACHE_PATH, DEFAULT_TRAIN_PARAMS); simplified function logic; improved documentation |
| tools/utils.py | Reordered imports to follow PEP 8 conventions |
| imageGen.py | Converted variables to uppercase constants; added validation assertions with descriptive messages; improved comments |
| imageGen_trace.py | Minor comment improvements |
| validateClassifier.ipynb | Changed quotes from single to double; removed execution counts |
| analysis_Similarity.ipynb | Formatting improvements for long lines |
| analysis_Dataset.ipynb | Formatting improvements for readability |
| analysis_Classifier.ipynb | Quote standardization and formatting |
| analysis_CLIP.ipynb | Formatting improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert ( | ||
| num_classes == 10 | ||
| ), f"Expected 10 classes, got {num_classes}" # 10 for imagenette/woof, 120 for stanford-dogs |
There was a problem hiding this comment.
The assertion hardcodes 10 classes, but the comment indicates this should support both 10 classes (for imagenette/woof) and 120 classes (for stanford-dogs). This will cause the script to fail when training on stanford-dogs dataset. Consider making this assertion dynamic based on the dataset being used, or removing the hardcoded value to allow flexibility across different datasets.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| assert ( | ||
| len(val_dataset.classes) == 10 | ||
| ), f"Expected 10 classes, got {len(val_dataset.classes)}" |
There was a problem hiding this comment.
The assertion hardcodes 10 classes, but the training dataset assertion comment (lines 118-120) indicates this should support both 10 classes (for imagenette/woof) and 120 classes (for stanford-dogs). This will cause validation to fail when training on stanford-dogs dataset. Consider making this assertion consistent with the training dataset validation.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@zhulinchng I've opened a new pull request, #3, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: zhulinchng <24189730+zhulinchng@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Remove hardcoded class count assertions to support multiple datasets
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@zhulinchng I've opened a new pull request, #4, to work on those changes. Once the pull request is ready, I'll request review from you. |
Remove hardcoded class count assertions to support multiple datasets
No description provided.