[ENH] Adding categorical support: Raising error in yes/no case#6732
[ENH] Adding categorical support: Raising error in yes/no case#6732fkiraly merged 64 commits intosktime:mainfrom
Conversation
(cherry picked from commit cc3d056)
(cherry picked from commit 2de7d8c)
Ref. sktime#5886 (review) (cherry picked from commit 2cecd6b)
(cherry picked from commit 1477012)
|
@fkiraly @yarnabrina please go through the above comment, I have mentioned the PRs this is dependent on. The main thing here is that all the logic is kept within the |
|
@fkiraly @yarnabrina I think this pr is ready now. |
yarnabrina
left a comment
There was a problem hiding this comment.
Good from my side. This RTD failure is irritating.
86407b1 to
c752b9f
Compare
fkiraly
left a comment
There was a problem hiding this comment.
test_skforecast_with_categorical seems to be failing with a genuine failure, could you kindly look into that?
I think it is soft dependency isolation not being carried out.
|
@fkiraly I've removed the base class change of moving the ignore-exog check from this pr. I will open a new pr for it soon. |
fkiraly
left a comment
There was a problem hiding this comment.
Thanks - fine from my side, then.
Reference Issues/PRs
Reference umbrella issue comment :#6109 (comment)
This is step 4 mentioned in that comment.
This PR depends on:
#6704 : Adding new categorical tag
#6490 : Feature_kind metadata
i.e this PR requires and contains commits of the above two PRs.
What does this implement/fix? Explain your changes.
Implementation of logic for handling categorical data and raising error appropriately.
Raising error in yes/no case is only in forecasters.
yes/no case - data has categorical , forecaster does not support.
In other modules(except classification), error is raised directly if categorical is present.
Does your contribution introduce a new dependency? If yes, which one?
No.
Did you add any tests for the change?
Yes.
PR checklist
For all contributions