Skip to content

Add some AccessApiOverload annotations #1031

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
Jan 28, 2025
Merged

Conversation

koperagen
Copy link
Collaborator

No description provided.

@koperagen koperagen force-pushed the acess-api-overload-1 branch from 5d55adf to 3769ca2 Compare January 27, 2025 14:55
@koperagen koperagen self-assigned this Jan 27, 2025
@@ -34,6 +34,7 @@ public interface SortDsl<out T> : ColumnsSelectionDsl<T> {

public fun String.desc(): SingleColumn<Comparable<*>?> = invoke<Comparable<*>>().desc()

@AccessApiOverload
Copy link
Collaborator

Choose a reason for hiding this comment

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

are we also deprecating extension functions on KProperty?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At least those from different column selection DSLs

@Jolanrensen Jolanrensen self-requested a review January 27, 2025 15:41
@Jolanrensen
Copy link
Collaborator

Do you actually also run a test somewhere to check whether the minimized dataframe-core still compiles? It could be that we use our public api somewhere internally

@@ -28,13 +28,16 @@ public interface DataRow<out T> {

public operator fun <R> get(expression: RowExpression<T, R>): R = expression(this, this)

@AccessApiOverload
public operator fun <R> get(column: ColumnReference<R>): R
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm maybe not exclude these ones yet? Let's say we have a DataRow<Any>, calling row["name"] provides no way of specifying the type. You can however do row[column<String>("name")] to get the type. (potentially even row["name"<String>()]) It would also suffice if we could get a typed row.get<String>("name") function I believe.

Copy link
Collaborator

Choose a reason for hiding this comment

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

related to #553

Copy link
Collaborator Author

@koperagen koperagen Jan 27, 2025

Choose a reason for hiding this comment

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

we have row.getValue<String>("name")

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah didn't notice it, as it was an extension function, great :)

@koperagen
Copy link
Collaborator Author

koperagen commented Jan 27, 2025

Do you actually also run a test somewhere to check whether the minimized dataframe-core still compiles? It could be that we use our public api somewhere internally

We use it, but it still compiles

@Jolanrensen
Copy link
Collaborator

Do you actually also run a test somewhere to check whether the minimized dataframe-core still compiles? It could be that we use our public api somewhere internally

We use it, but it still compiles

oh right, you only change visibility, very cool :)

@Jolanrensen Jolanrensen self-requested a review January 28, 2025 11:26
@koperagen koperagen merged commit b3306cb into master Jan 28, 2025
5 checks passed
@koperagen koperagen added this to the 0.16 milestone Jan 29, 2025
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.

3 participants