Skip to content

[ENH] Fix typos, improve exception handling, and update warning stacklevel in BaseModel and encoders#2210

Open
Kayd-06 wants to merge 1 commit intosktime:mainfrom
Kayd-06:fix/typos-and-warnings
Open

[ENH] Fix typos, improve exception handling, and update warning stacklevel in BaseModel and encoders#2210
Kayd-06 wants to merge 1 commit intosktime:mainfrom
Kayd-06:fix/typos-and-warnings

Conversation

@Kayd-06
Copy link

@Kayd-06 Kayd-06 commented Mar 18, 2026

Description:

This Pull Request addresses several small but important code quality issues in the pytorch_forecasting library. These changes improve code maintainability, ensure robust error handling during model initialization, and significantly enhance the usability of library warnings by providing better traceability for end-users.

Key Changes:

  1. Typo Corrections in BaseModel:
    • Renamed the internal variable monotinicity_loss to monotonicity_loss throughout _base_model.py and its comments to align with standard naming conventions.
    • Corrected a typo in a source code comment: # add additionl loss -> # add additional loss.
  2. Robust Exception Handling:
    • Refactored a bare except: clause in BaseModel.__init__ to except Exception:. This change prevents catching system-level exceptions while still defensively skipping individual non-serializable hyperparameters to prevent training crashes.
  3. Enhanced Warning Traceability:
    • Added stacklevel=3 to warnings.warn calls in _base_model.py and encoders.py.
    • By increasing the stacklevel, these warnings now correctly point to the user's code (e.g., the line where encoder.transform() is called) rather than pointing to the internal library file. This follows standard Python library best practices (similar to pandas or numpy).

Did you add any tests for the change?

  • Manual Verification: Created a specialized verification script to trigger a UserWarning in NaNLabelEncoder. Confirmed the warning traceback correctly identifies the caller's line number after the fix.
  • Regression Testing: Ran the full test suite for TemporalFusionTransformer (tests/test_models/test_temporal_fusion_transformer.py). All 38 tests passed (excluding one pre-existing numerical precision assertion) confirming no functional regressions in BaseModel logic.

PR checklist:

  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG].
  • Added/modified tests (Verified with existing model tests and manual traceability tests).
  • Used pre-commit hooks when committing.

@codecov
Copy link

codecov bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@edbdeb4). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2210   +/-   ##
=======================================
  Coverage        ?   86.62%           
=======================================
  Files           ?      165           
  Lines           ?     9736           
  Branches        ?        0           
=======================================
  Hits            ?     8434           
  Misses          ?     1302           
  Partials        ?        0           
Flag Coverage Δ
cpu 86.62% <100.00%> (?)
pytest 86.62% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant