scikit-learn / enhancement_proposals

Enhancement proposals for scikit-learn: structured discussions and rational for large additions and modifications
https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
48 stars 34 forks source link

Questions and comments about SLEP006 -- Sample props #45

Closed NicolasHug closed 4 years ago

NicolasHug commented 4 years ago

1.

we can consider the use of keys that are not limited to strings or valid identifiers (and hence are not limited to using _ as a delimiter).

I don't understand how we are currently limited to using _ as a delimiter. Should this be __? But even then I still don't follow.

2.

TODO: proceed from here. Note that this change implies the need to add a parameter to unwrap_X, since we will now append an additional column to X.

I don't understand why we don't need to add an additional column to X in this case.

3.

while a GridSearchCV wrapping a Pipeline currently takes parameters with keys like {step_name}{prop_name}, this explicit routing, and conflict with GridSearchCV routing destinations, implies keys like estimator__{step_name}{prop_name}.

I'm not sure I completely understand this. A small illustration might help? Also, does "GridSearchCV routing destinations" refers to param_grid instead?

4.

All consumers would be required to check that

This sentence seems unfinished?

5.

Here the meta-estimator provides only what its each of its children requests. The meta-estimator would also need to request, on behalf of its children, any prop that descendant consumers require.

I could be wrong, but this doesn't seem to define the "essence" of solution 4. It seems to me that in solution 3, as well, the meta estimators only provides what the childrens need/request?

NicolasHug commented 4 years ago

Sorry pressed too soon, closing, don't want to open this now