Hotfix for no dependency config loading#387
Conversation
kozlov721
requested review from
conorsim,
klemen1999 and
tersekmatija
and removed request for
a team
|
Warning Review limit reached
More reviews will be available in 42 minutes and 48 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR modifies ChangesMetrics Generation Exception Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
luxonis_train/config/predefined_models/base_predefined_model.py (1)
222-252:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSuppressed exceptions still lead to a crash via
Nonereturn.If any exception occurs in this block, it is swallowed and
_generate_metrics()returnsNoneimplicitly, which later breaks atmetrics + [...]innodeswithTypeError. Return a safe fallback list when override resolution fails.Proposed fix
- with suppress(Exception): - task = NODES.get(self._head).task - metrics = [] - applied_per_class_override = False - - for metric in self._metrics: - metric_params = dict(self._metrics_params) - metric_cls = METRICS.get(metric) - aliases = metric_cls.get_predefined_model_params_aliases(task) - param_name = aliases.get("per_class_metrics") - if param_name is not None: - metric_params[param_name] = self._per_class_metrics - applied_per_class_override = True - - metrics.append( - MetricModuleConfig( - name=metric, - params=metric_params, - is_main_metric=metric == self._main_metric, - ) - ) - - if self._metrics and not applied_per_class_override: - logger.warning( - "Ignoring `per_class_metrics` for predefined model metrics " - f"{self._metrics} because none of them support a per-class " - "override." - ) - - return metrics + try: + task = NODES.get(self._head).task + metrics = [] + applied_per_class_override = False + + for metric in self._metrics: + metric_params = dict(self._metrics_params) + metric_cls = METRICS.get(metric) + aliases = metric_cls.get_predefined_model_params_aliases(task) + param_name = aliases.get("per_class_metrics") + if param_name is not None: + metric_params[param_name] = self._per_class_metrics + applied_per_class_override = True + + metrics.append( + MetricModuleConfig( + name=metric, + params=metric_params, + is_main_metric=metric == self._main_metric, + ) + ) + + if self._metrics and not applied_per_class_override: + logger.warning( + "Ignoring `per_class_metrics` for predefined model metrics " + f"{self._metrics} because none of them support a per-class " + "override." + ) + return metrics + except Exception as exc: + logger.warning( + "Failed to apply `per_class_metrics` override; falling back to " + f"default metric params. Reason: {exc}" + ) + return [ + MetricModuleConfig( + name=metric, + params=self._metrics_params, + is_main_metric=metric == self._main_metric, + ) + for metric in self._metrics + ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@luxonis_train/config/predefined_models/base_predefined_model.py` around lines 222 - 252, The try/suppress block in _generate_metrics (the shown code that reads NODES.get(self._head).task, iterates self._metrics, uses METRICS.get and builds MetricModuleConfig) swallows exceptions and allows the function to implicitly return None, causing downstream TypeError; change the suppression to catch Exception explicitly, log the error, and return a safe fallback list (e.g., an empty list or a default MetricModuleConfig list) so callers combining metrics (metrics + [...]) always receive a list; ensure you reference the same symbols (NODES, METRICS, MetricModuleConfig, self._metrics, self._metrics_params, self._per_class_metrics, self._main_metric) when implementing the fallback and logging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@luxonis_train/config/predefined_models/base_predefined_model.py`:
- Around line 222-252: The try/suppress block in _generate_metrics (the shown
code that reads NODES.get(self._head).task, iterates self._metrics, uses
METRICS.get and builds MetricModuleConfig) swallows exceptions and allows the
function to implicitly return None, causing downstream TypeError; change the
suppression to catch Exception explicitly, log the error, and return a safe
fallback list (e.g., an empty list or a default MetricModuleConfig list) so
callers combining metrics (metrics + [...]) always receive a list; ensure you
reference the same symbols (NODES, METRICS, MetricModuleConfig, self._metrics,
self._metrics_params, self._per_class_metrics, self._main_metric) when
implementing the fallback and logging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe7785eb-1059-4e0b-98d9-00dde0c9291a
📒 Files selected for processing (1)
luxonis_train/config/predefined_models/base_predefined_model.py
1 participant
Purpose
Falls back to disabled per-class metrics in case of an error instead of crashing.
This solves crashing no-dependency config loading in HubAI.
Specification
None / not applicable
Dependencies & Potential Impact
None / not applicable
Deployment Plan
None / not applicable
Testing & Validation
None / not applicable
AI Usage
Assisted-by: AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2]
Submitted code was reviewed by a human: YES/NO
The author is taking the responsibility for the contribution: YES/NO
Summary by CodeRabbit