sklearn converter does not work with new columntransformer #494

Activity

1. mfeurer commented on Jul 27, 2018

   

   mfeurer

   on Jul 27, 2018

   Collaborator

   More actions

   which we probably want to use for the benchmark study

   Will OpenML support having multiple instantiations of the same flow by then?

   And yes, we should fix this. I'll ask @ArlindKadra once everything else related to the benchmark study is done.

   👍React with 👍1Reacted by Arlind Kadra
2. janvanrijn commented on Sep 12, 2018

   

   janvanrijn

   on Sep 12, 2018

   MemberAuthor

   More actions

   This will place a huge dependency on the scikit-learn version btw. Next week, we should really talk about decoupling openml-python and openml-scikitlearn
3. janvanrijn commented on Sep 12, 2018

   

   janvanrijn

   on Sep 12, 2018

   MemberAuthor

   More actions

   Something that came up in a conversation between @amueller and me.

   In some way, it seemed weird that (de-)serializing a ColumnTransformer crashed. It seemed like a normal estimator, and should be serialized as such.

   The reason that it crashed is that the openml-python converter assumes that all steps of a pipeline, voting classifier or feature union are lists of tuples of length 2. This is no longer the case for the ColumnTransformer, it is a list of tuples of length 3.

   Fixing this is rather easy, although my current headache is how to fix this without further complicating the code. Please expect an PR very soon.
4. janvanrijn commented on Sep 12, 2018

   

   janvanrijn

   on Sep 12, 2018

   MemberAuthor

   More actions

   I committed a first attempt. I don't like it for the following reasons:

   1. why does it serialize the name to:
      sklearn.compose._column_transformer.ColumnTransformer(numeric=sklearn.preprocessing.data.StandardScaler, nominal=sklearn.preprocessing._encoders.OneHotEncoder)
      I didn't expect _column_transformer, data and _encoders there.

   2. The columns on which is splits are now part of the flow definition. (should be part of the setup definition, IMO)

   3. We need to have aliases for all components that are used double, but I guess we already knew that.
5. mfeurer commented on Sep 13, 2018

   

   mfeurer

   on Sep 13, 2018

   Collaborator

   More actions

   Could you please post some example code?
6. janvanrijn commented on Sep 13, 2018

   

   janvanrijn

   on Sep 13, 2018

   MemberAuthor

   More actions

   Something like this?

   import sklearn.componse
   import sklearn.preprocessing
   model = sklearn.compose.ColumnTransformer(
   transformers=[
                   ('numeric', sklearn.preprocessing.StandardScaler(), [0, 1, 2]),
                   ('nominal', sklearn.preprocessing.OneHotEncoder(handle_unknown='ignore'), [3, 4, 5])],
               remainder='passthrough')
   fixture_name = 'sklearn.compose.ColumnTransformer(' \
                          'numeric=sklearn.preprocessing.StandardScaler,' \
                          'nominal=sklearn.preprocessing.OneHotEncoder)'
   serialization = sklearn_to_flow(model)
   self.assertEqual(serialization.name, fixture_name)
   
7. janvanrijn commented on Sep 13, 2018

   

   janvanrijn

   on Sep 13, 2018

   MemberAuthor

   More actions

   I was btw wrong about shortcoming 1 and 2.

   Shortcoming 1 is something that happens now for all test cases (all fixtures changed?) and shortcoming 2 is simply not true (my bad).

   In order for us to merge this, we would need:

   1. final release for sklearn 0.20.0 (dependency check requires a real release number)
   2. make a decision about unit tests. many of them have specific assumptions that require a specific sklearn version (e.g., fixture names, number of parameters of pipeline, etc)
8. janvanrijn commented on Sep 13, 2018

   

   janvanrijn

   on Sep 13, 2018

   MemberAuthor

   More actions

   @mfeurer can we free some time for this on the workshop? There are many difficulties, mostly regarding how to handle version differences in test setup.
9. mfeurer commented on Nov 16, 2018

   

   mfeurer

   on Nov 16, 2018

   Collaborator

   More actions

   This was added in #523 and #526.
10. mfeurer

    closed this as completedon Nov 16, 2018