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

Rename to camelCase support in compiler plugin #1029

Merged
merged 6 commits into from
Jan 28, 2025

Conversation

Jolanrensen
Copy link
Collaborator

Fixes #1025

Adds compiler plugin support for DataFrame.renameToCamelCase() and DataFrame.rename {}.toCamelCase()

Got some help from Junie :) The first function was almost entirely written by AI, the second one needed a more guiding hand from me.

See commit messages for more specifics

@Jolanrensen Jolanrensen added the Compiler plugin Anything related to the DataFrame Compiler Plugin label Jan 27, 2025
@Jolanrensen Jolanrensen added this to the 0.16.0 milestone Jan 27, 2025
@Jolanrensen Jolanrensen requested a review from koperagen January 27, 2025 13:09
class RenameToCamelCaseClause : AbstractSchemaModificationInterpreter() {
val Arguments.receiver: RenameClauseApproximation by arg()

override fun Arguments.interpret(): PluginDataFrameSchema {
Copy link
Collaborator

Choose a reason for hiding this comment

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

i'd suggest convert selected columns to path and receiver.df.asDataFrame().rename { paths }.toCamelCase().toPluginDataFrameSchema()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's easier indeed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

and it promotes the idea that the original function solves the compiler plugin part :) which is great. I just need to become a bit more comfortable with the helper functions available to me in the compiler plugin module, but I'll get there :)

)
val renamed = df.renameToCamelCase()

// Verify extension properties
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is redundant here, no point testing runtime behaviour

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not entirely, it's still worth checking that the generated property actually has the value we expect it to have. Could catch a potential bugs like types not being recognized or accessors mismatching the correct columns

return SimpleFrameColumn(
name,
f(columns(), nextName, selected, path)
internal fun SimpleFrameColumn.map(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please, for easy and fast reviews let's hold changes related to linting and other non-functional changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I put it in a separate commit, so you can skip those if you wish

Copy link
Collaborator

@koperagen koperagen left a comment

Choose a reason for hiding this comment

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

as mentioned in the comments

@Jolanrensen Jolanrensen force-pushed the rename-to-camelcase-2 branch from 88599a7 to cd5888c Compare January 27, 2025 18:53
@Jolanrensen Jolanrensen merged commit b4739b9 into master Jan 28, 2025
5 checks passed
@Jolanrensen Jolanrensen deleted the rename-to-camelcase-2 branch January 28, 2025 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compiler plugin Anything related to the DataFrame Compiler Plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for renameToCamelCase in Compiler plugin
2 participants