fix: always set class_names from dataset annotations#518
Draft
lixiaolei1982 wants to merge 2 commits intoroboflow:developfrom
Draft
fix: always set class_names from dataset annotations#518lixiaolei1982 wants to merge 2 commits intoroboflow:developfrom
lixiaolei1982 wants to merge 2 commits intoroboflow:developfrom
Conversation
- Remove conditional check that only set class_names when None - Unconditionally assign dataset-derived class_names to train_config - Ensure consistency between dataset annotations and training configuration
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a logic flaw in the train_from_config method where class names from the dataset annotations were only conditionally assigned to the training configuration. The change ensures that class names derived from the dataset are always used, maintaining consistency between dataset annotations and training configuration.
Changes:
- Remove conditional check that only set class_names when it existed and was None
- Unconditionally assign dataset-derived class_names to train_config
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
60b16c1 to
523f9df
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Member
|
@lixiaolei1982, could you please elaborate on why the actual behaviour is wrong? |
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.
Description
The current code in train_from_config method contains an incorrect conditional check:
if "class_names" in train_config and train_config["class_names"] is None:
train_config["class_names"] = class_names
This logic is flawed because:
1.It only sets class_names when it already exists in train_config AND is None
2.It doesn't handle the case where class_names might not exist in train_config at all
3.It doesn't override potentially incorrect class_names values that are non-None
The correct behavior should be to always set train_config["class_names"] to the class_names variable derived from the dataset (Roboflow or COCO), ensuring consistency between the dataset annotations and the training configuration.
#Changes:
Remove the conditional check and unconditionally set train_config["class_names"] = class_names
This ensures the class names from the actual dataset annotations are always used for training
#Impact:
Fixes potential mismatches between dataset classes and training configuration
Ensures consistent behavior regardless of initial train_config["class_names"] value
Maintains backward compatibility while fixing the logic error