To type overloads with diff nullability#1839
Conversation
f23c7ed to
325ac42
Compare
| assertNullabilityPreserved(converted, typeOf<IMG>()) | ||
| } | ||
|
|
||
| // @Test |
There was a problem hiding this comment.
toUrl/toURL throw NoClassDefFoundError. Seems like they have the same signature which causes a conflict in case insensitive environment
| * @return A new [DataFrame] with the values converted to [LocalDate]. | ||
| */ | ||
| @JvmName("toLocalDateFromTInt") | ||
| @JvmName("toLocalDateFromTIntNullable") |
There was a problem hiding this comment.
Should it be corrected like that?
| @Refine | ||
| @Converter(IMG::class, nullable = false) | ||
| @Interpretable("ToSpecificType") | ||
| public fun <T, R : URL?> Convert<T, URL>.toImg(width: Int? = null, height: Int? = null): DataFrame<T> = |
There was a problem hiding this comment.
Do we need the R?
There was a problem hiding this comment.
I think R should have been the second argument in Convert<>
| toJavaLocalDate(formatter = pattern?.let { DateTimeFormatter.ofPattern(pattern) }, locale = locale) | ||
| .convert(this.columns).toLocalDate() | ||
|
|
||
| @Deprecated( |
There was a problem hiding this comment.
I couldn't test these last 3 deprecated functions: when I use it in a test, the compile time schema of the converted dataframe contains String and String?. I didn't investigate it further because first I wanted to ask if we need to test them at all :)
There was a problem hiding this comment.
Hmm, they seem to miss compiler plugin annotations, not sure why
There was a problem hiding this comment.
it's ok to not test them
Fixes #1695
However, after the removal of the overloads, the compiler plugin produces the expected schema only since the version 2.4.0. So it's probably better to postpone merging this PR till the 2.4.0 release.