Skip to content

Conversation

AndreiKingsley
Copy link
Collaborator

Fixes #1028.

@AndreiKingsley AndreiKingsley marked this pull request as ready for review October 2, 2025 14:12
@Deprecated(MESSAGE_SHORTCUT_1_0, ReplaceWith("into { pathOf(column) }"), DeprecationLevel.ERROR)
@Refine
@Interpretable("MoveInto0")
public fun <T, C> MoveClause<T, C>.into(column: String): DataFrame<T> = pathOf(column).let { path -> into { path } }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you remember why we want to remove it? It's supported by compiler plugin, seems useful

Copy link
Collaborator

Choose a reason for hiding this comment

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

is it just rename {}.into {} maybe? or move {}.under("")?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's move { myCol }.toTop().rename { myCol }.into("newName")

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, I really don't see a reason 😄 . Let's keep it.

// region DataFrame

@CandidateForRemoval
@Deprecated(MESSAGE_SHORTCUT_1_0, ReplaceWith("columns().toDataFrame().cast()"), DeprecationLevel.ERROR)
Copy link
Collaborator

Choose a reason for hiding this comment

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

we also usually put the replaceWith contents in the deprecations file. I'm impartial to it, as long as we have at least one constant String there to track the deprecations

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 don't like it, because it's hard to validate when it's remote, especially in singular cases + losing IDE code analysis.
And btw we have a lot of them in the project:
image

@Jolanrensen
Copy link
Collaborator

should we immediately go to ERROR? I don't think many of these overloads hurt. So while we will clean them up, I wouldn't say there's much hurry, so WARNING could suffice

col.type() shouldBe typeOf<AnyRow>()
col.kind() shouldBe ColumnKind.Group
col[0] shouldBe row
@Suppress("DEPRECATION_ERROR")
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we deprecate them, we should probably remove any internal uses

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for DataRow.isEmpty(), I'd make it internal after deprecation and public removal for such tests.

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.

Candidates for removal
3 participants