Skip to content

Hide properties of intermediate objects and remove data class attributes #828

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

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

koperagen
Copy link
Collaborator

@koperagen koperagen commented Aug 20, 2024

In the image you can see how it used to be for some of those objects. copy, properties, generated component functions. These are useless, and yet they appear in the completion.
I left "data" for those classes where implementation uses copy more than once, and for some reason component* functions do not appear for Gather and Merge, so idk.
toString for now is just the same as it was with data class, but in theory it can even be improved in the future for better debugger experience.

df.convert { a }.

image

@koperagen koperagen self-assigned this Aug 20, 2024
@koperagen koperagen requested a review from Jolanrensen August 20, 2024 13:56
@koperagen koperagen added this to the 0.14.0 milestone Aug 20, 2024
@Jolanrensen
Copy link
Collaborator

Jolanrensen commented Aug 20, 2024

It's a good idea, but we need to be careful changing data classes to normal classes. There might be parts which rely on the equality of those classes being based solely on its components. Removing data makes two classes with the same arguments not equal anymore. (this can pop up in strange places, such as Sets, tests etc)

Also, is this safe from a jupyter standpoint? aka, can we still access the internal df value, even if jupyter is moved to another module?

@koperagen
Copy link
Collaborator Author

Also, is this safe from a jupyter standpoint? aka, can we still access the internal df value, even if jupyter is moved to another module?

Migrating jupyter to a different module requires it to use friend module / suppress hidden members regardless of that

It's a good idea, but we need to be careful changing data classes to normal classes. There might be parts which rely on the equality of those classes being based solely on its components. Removing data makes two classes with the same arguments not equal anymore. (this can pop up in strange places, such as Sets, tests etc)

All tests are green and it doesn't seem like intermediate objects are used anywhere except respective terminal operations. That's why i decided to not generate hashCode + equals. I think they were "data" not for this, but for "copy" and "toString", and other things are just a side effect

@koperagen koperagen force-pushed the multiplex-objects-public-api branch from f0fdf02 to f7251dd Compare August 20, 2024 14:28
Copy link
Contributor

Generated sources will be updated after merging this PR.
Please inspect the changes in here.

@koperagen koperagen merged commit 5d88bb2 into master Aug 20, 2024
4 of 5 checks passed
@koperagen koperagen deleted the multiplex-objects-public-api branch August 26, 2024 13:47
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.

2 participants