[ENH] Replace asserts with proper if else Exception handling#1589
[ENH] Replace asserts with proper if else Exception handling#1589Omswastik-11 wants to merge 9 commits intoopenml:mainfrom
asserts with proper if else Exception handling#1589Conversation
|
Hi @fkiraly !! I would like to know you suggestion on this .
In |
asserts with proper if else error handlingasserts with proper if else Exception handling
fkiraly
left a comment
There was a problem hiding this comment.
Yes, looks reasonable for me for a first go, to isolate the if/else in a function.
More generally, from an architecture standpoint however, whenever I see a long list of if/elses on the argument class type, I think it is too high coupling and it should be either a method of the (task/argument) class or a visitor pattern.
Why: imagine you want to add a new task. Now you have to add another elif in all of these functions. This is absolutely not extensible.
This should be refactored - I would say, in another PR, after opening an issue with a plan - so the task itself, or a visitor, manages whatever is in the if/else.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1589 +/- ##
==========================================
+ Coverage 52.04% 52.67% +0.62%
==========================================
Files 36 36
Lines 4333 4342 +9
==========================================
+ Hits 2255 2287 +32
+ Misses 2078 2055 -23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
asserts with proper if else Exception handlingasserts with proper if else Exception handling

Fixes #1581