Skip to content
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

Clarify the situation when generated extension property causes ClassCast or NPE exceptions #965

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

koperagen
Copy link
Collaborator

Now if compiler plugin or jupyter integration has a bug and generated property returns type that mismatches runtime, we get vague ClassCast or NPE. This PR tries to give a little bit more info about it and propose a workaround

@koperagen koperagen added the bug Something isn't working label Nov 25, 2024
@koperagen koperagen added this to the 0.16.0 milestone Nov 25, 2024
@koperagen koperagen self-assigned this Nov 25, 2024
@Jolanrensen
Copy link
Collaborator

If I'm correct, you fixed it just for convert.to, right?
However the ClassCastException can occur any time a value from a column is accessed, like:

val ColumnsContainer<Marker>.a get() = this["a"] as DataColumn<String>
val DataRow<Marker>.a get() = this["a"] as String

df.filter { a != "something" } // throws ClassCastException if `a` does not contain Strings
val values = df.a.values()
values[0] // throws ClassCastException if `a` does not contain Strings (not sure if this can be avoided)

@koperagen
Copy link
Collaborator Author

For convert.with
Yes, there are other cases, but for example this one is a bit better. Because this["a"] as String is checked cast, so it will fail right there and with sensible stack trace.

val DataRow<Marker>.a get() = this["a"] as String
df.filter { a != "something" } // throws ClassCastException if `a` does not contain Strings

When DataColumn accessors are called, it's unchecked cast and value can travel quite a bit before library "extract" value from the column. So stack trace and exception message are less helpful - i think convert with is popular example of such operation

@Jolanrensen
Copy link
Collaborator

@koperagen I see :) Well, then convert.with throwing the new exception is a good start :) We'll likely need to add it in other places as well, but maybe that can be done later after we discover where else it pops up.

@koperagen koperagen force-pushed the buggy-extension-property-exception branch from 7eaf070 to 70432be Compare November 25, 2024 19:32
@Jolanrensen Jolanrensen self-requested a review November 26, 2024 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants