[Minor] [ML] When trainingSummary is None, it should throw RuntimeException. #11784
Conversation
|
Test build #53432 has finished for PR 11784 at commit
|
| def summary: LogisticRegressionTrainingSummary = trainingSummary.getOrElse { | ||
| throw new SparkException( | ||
| "No training summary available for this LogisticRegressionModel", | ||
| new RuntimeException()) |
There was a problem hiding this comment.
Generally you don't use RuntimeException directly... why is the exception chained anyway and what was wrong with NPE? just kinda inaccurate? NoSuchElementException is what None.get throws.
There was a problem hiding this comment.
Yes, I agree we usually don't use RuntimeException directly. But I think it's inappropriate use NullPointerException here, because NullPointerException was thrown when an application attempts to access null in a case. Here we throw the SparkException just want to handle the case that trainingSummary == None, we did not access null actually. Should we just throw an SparkException?
There was a problem hiding this comment.
It already threw SparkException though, so I wasn't sure why it was worth changing. However, I also can't figure out the point of that chained NPE. This seems OK.
|
Test build #53514 has finished for PR 11784 at commit
|
|
Merged to master |
…ption. ## What changes were proposed in this pull request? When trainingSummary is None, it should throw ```RuntimeException```. cc mengxr ## How was this patch tested? Existing tests. Author: Yanbo Liang <ybliang8@gmail.com> Closes apache#11784 from yanboliang/fix-summary.
What changes were proposed in this pull request?
When trainingSummary is None, it should throw
RuntimeException.cc @mengxr
How was this patch tested?
Existing tests.