Skip to content

1662 cannot pickle preprocessingordinalencoder instance (#1759)#1774

Open
kulbachcedric wants to merge 2 commits intomainfrom
1662-cannot-pickle-preprocessingordinalencoder-instance
Open

1662 cannot pickle preprocessingordinalencoder instance (#1759)#1774
kulbachcedric wants to merge 2 commits intomainfrom
1662-cannot-pickle-preprocessingordinalencoder-instance

Conversation

@kulbachcedric
Copy link
Copy Markdown
Contributor

No description provided.

@kulbachcedric kulbachcedric linked an issue Mar 28, 2026 that may be closed by this pull request
@MaxHalford
Copy link
Copy Markdown
Member

@kulbachcedric normally we have systematic unit tests to check each class can be picked. Could you take a look at why this class wasn't tested? There must be something missing in the test suite.

* Add SkipCounter class

* Remove class

* Check every model for pickling
@kulbachcedric kulbachcedric marked this pull request as ready for review March 29, 2026 13:53
Comment on lines +148 to +149
def check_pickling_supports_roundtrip(model):
assert isinstance(pickle.loads(pickle.dumps(model)), model.__class__)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have check_pickling. Can you check why it's not running OrdinalEncoder?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have check_pickling but it takes a model and a dataset (not only a model) and that's why the OrdinalEncoder was not checked. As I quick fix, I added the check_pickling_supports_roundtrip method

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok good point. Doesn't hurt adding this test anyway

@MaxHalford
Copy link
Copy Markdown
Member

An update to ruff was merged, so you have some conflicts to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot pickle preprocessing.OrdinalEncoder instance

2 participants