MNT support older cython types' reduce#453
Conversation
|
@TamaraAtanasoska @BenjaminBossan this actually fixes the cython issues. WDYT? |
|
I'm loving my |
BenjaminBossan
left a comment
BenjaminBossan
left a comment
There was a problem hiding this comment.
Thanks for this update, I just added a small comment.
| def loss_get_state(obj: Any, save_context: SaveContext) -> dict[str, Any]: | ||
| state = reduce_get_state(obj, save_context) | ||
| state["__loader__"] = "LossNode" | ||
| reduce = obj.__reduce__() |
There was a problem hiding this comment.
Maybe another name as reduce used to be builtin (and there is still functools.reduce)?
Also: If the Cython version is too low, would the obj.__reduce__() fail? If this could realistically happen to users, the error could be intercepted and reraised with the message that users should update Cython to version x.y.z.
There was a problem hiding this comment.
For now we are running this code for known objects, and since Cython is a build dependency and the installed cython doesn't matter, and that we use modern cython in sklearn build, we're fine.
But if we start supporting arbitrary objects via reduce for cython, then this would be an issue.
2 participants
Cython at some point automatically started to add
__reduce__to objects, which is not too bad.This also adds tests for sklearn versions 1.4 and 1.3