-
Notifications
You must be signed in to change notification settings - Fork 28.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-31853][DOCS] Mention removal of params mixins setter in migration guide #28663
[SPARK-31853][DOCS] Mention removal of params mixins setter in migration guide #28663
Conversation
@zhengruifeng FYI |
Dumb question, but does that affect callers? |
Code that used to call into the setters, e.g. |
@EnricoMi The old mixins provided some wrong setters in the params, (e.g. For the callers like @huaxingao @zero323 How do think about this? |
I don't think the changes in SPARK-29093 affect users. Using
It doesn't matter to the users if |
The In my opinion, it does not matter if this is common usage or not, it is a possible usage that breaks with this change. First place to look when your code breaks is the migration guide. |
Yeah the only downside is that if we fill the migration guide with a bunch of niche stuff, it might be hard to figure out what the important common items are. I'm not sure how much user code uses the mixins directly, but if there is any code that does it's probably something like |
Jenkins test this please |
Test build #123492 has finished for PR 28663 at commit
|
…ion guide ### What changes were proposed in this pull request? The Pyspark Migration Guide needs to mention a breaking change of the Pyspark ML API. ### Why are the changes needed? In SPARK-29093, all setters have been removed from `Params` mixins in `pyspark.ml.param.shared`. Those setters had been part of the public pyspark ML API, hence this is a breaking change. ### Does this PR introduce _any_ user-facing change? Only documentation. ### How was this patch tested? Visually. Closes #28663 from EnricoMi/branch-pyspark-migration-guide-setters. Authored-by: Enrico Minack <[email protected]> Signed-off-by: Sean Owen <[email protected]> (cherry picked from commit 4bbe3c2) Signed-off-by: Sean Owen <[email protected]>
Merged to master/3.0 |
What changes were proposed in this pull request?
The Pyspark Migration Guide needs to mention a breaking change of the Pyspark ML API.
Why are the changes needed?
In SPARK-29093, all setters have been removed from
Params
mixins inpyspark.ml.param.shared
. Those setters had been part of the public pyspark ML API, hence this is a breaking change.Does this PR introduce any user-facing change?
Only documentation.
How was this patch tested?
Visually.