Improved parameter validation for TorchForecastingModel

[tRosenflanz]

Fixes #2843

Summary

Simple improvement to consider parameters of the full class inheritance of the checked class instead of specific classes for TorchForecastingModel

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.45%. Comparing base (2c666f8) to head (19ac6f6).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2908      +/-   ##
==========================================
- Coverage   95.52%   95.45%   -0.07%     
==========================================
  Files         146      146              
  Lines       15703    15710       +7     
==========================================
- Hits        15000    14996       -4     
- Misses        703      714      +11     

☔ 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.

[tRosenflanz]

Do you want me to add a test for this? I can use the example from the original issue for it

[dennisbader]

Hi @tRosenflanz and thanks a lot for the PR 🚀

Yes, a unit test would be great :) Could you also:

• try to improve the code coverage with the new tests (from the codecov comment above).
• add an entry to CHANGELOG.md describing your change

[tRosenflanz]

Can I rework the test_wrong_model_creation_params using a RnnLambda class or would you prefer a separate test? They will be testing essentially the same behavior so I reworked the existing test but had to change the params because inheritance changes the argument order

Also I will remove that uncovered Try/except block - it only comes into play with very exotic cases that should probably be forbidden anyway like using double inheritance with C implemented types like list

[dennisbader]

Hi @tRosenflanz, removing the try / except sounds good. Probably better to let it fail then continue silently and we won't know what went wrong.

I would prefer a new test. You can pretty much copy-paste the test_wrong_model_creation_params, and adapt it to use a Subclass that adds one additional parameter. You can then add this param to the existing valid_kwarg and test that it works. invalid_kwarg should still raise an exception.

[dennisbader]

Perfect, thanks a lot @tRosenflanz, this really simplifies subclassing from TFM 🚀

I pushed a minor change to also test that passing the positional arguments works.