[ENH] skbase clone vs sklearn clone - comparison and actions

[fkiraly]

posted by @tpvasconcelos in #275 - opening a new issue to discuss design and actions arising, if any

---

Just for reference, I'm attaching a visual diff between the current BaseObject._clone() implementation and sklearn's _clone_parametrized():

Summary of differences:

1. skbase does not clone dict values (@fkiraly do you know why?)
2. sklearn also clones the _metadata_request and _sklearn_output_config attributes
3. skbase now also clones the config flags

I need to think more about this to give any meaningful input here.

Either way, some tech discussions on uniformization of APIs are going to be scheduled for later this year between the two projects (sktime, sklearn) - in case you are interested.

@fkiraly definitely

Originally posted by @tpvasconcelos in #275 (comment)

[fkiraly]

Summary of differences:

These are likely due to changes that were merged in sklearn since the skbase _clone was copied from there.

The forking was conscious, as discussed in #275 - for instance, the more recently added _sklearn_output_config in sklearn is better handled by set_config/get_config - they keep proliferating attrs that are really hard to keep track of if you are not an sklearn core dev...

1. skbase does not clone dict values (@fkiraly do you know why?)

The simple reason is that sklearn did not support it back then. Seems like a reasonable idea to add.

2. sklearn also clones the _metadata_request and _sklearn_output_config attributes

Yes, this stuff was added recently in sklearn, see above. This is too specific to sklearn estimators or pipelines imo to be part of a super-base class, and imo one example where sklearn has recently gone down a rabbit hole of decreasing cohesion as a side effect of adding more and more features - in particular, features that sklearn-like packages don't necessarily need - and then having to deal with them all over the place.

For instance, sth like get_config / set_config is useful, but _sklearn_metadata_config is extremely package specific, and each such addition makes it less and less easy to use sklearn BaseEstimator as a starting point for a sklearn-like package that is not identical with sklearn interfaces.

Feels a bit like a gradual "Lasagna-fication" of once crisp architecture...

3. skbase now also clones the config flags

Yes, given that it happens in the same place where the sklearn config is being dealt with validates your approach, because the same solution was found independently twice.

[fkiraly]

#281 improves the structure, but does not add support for dict