Skip to content

To type overloads with diff nullability#1839

Draft
Allex-Nik wants to merge 1 commit into
masterfrom
to-type-overloads-diff-nullability
Draft

To type overloads with diff nullability#1839
Allex-Nik wants to merge 1 commit into
masterfrom
to-type-overloads-diff-nullability

Conversation

@Allex-Nik
Copy link
Copy Markdown
Collaborator

@Allex-Nik Allex-Nik commented May 4, 2026

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.

@Allex-Nik Allex-Nik requested a review from koperagen May 4, 2026 08:36
@Allex-Nik Allex-Nik force-pushed the to-type-overloads-diff-nullability branch from f23c7ed to 325ac42 Compare May 8, 2026 10:14
assertNullabilityPreserved(converted, typeOf<IMG>())
}

// @Test
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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")
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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> =
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Do we need the R?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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(
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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 :)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm, they seem to miss compiler plugin annotations, not sure why

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it's ok to not test them

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.

Remove redundant toType overloads with different nullability in convert API

3 participants