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

Nullable accessors and fixed convertTo #175

Merged
merged 8 commits into from
Oct 10, 2022
Merged

Conversation

Jolanrensen
Copy link
Collaborator

@Jolanrensen Jolanrensen commented Oct 5, 2022

Let's say you have a Dataframe of Location:

@DataSchema
public data class Location(
    val name: String,
    val gps: Gps?,
)

@DataSchema
public data class Gps(
    val latitude: Double,
    val longitude: Double,
)

val locations: DataFrame<Location> = listOf(
    Location("Home", Gps(0.0, 0.0)),
    Location("Away", null),
).toDataFrame()

In the current implementation working with gps is very difficult. Since it is generated as DataRow<Gps?>, the extension accessors like latitude and longitude don't work. Plus, in the dataframe, the gps property is shown as gps: {latitude:Double?, longitude:Double?}, which further shows that the accessor would be helpful.
The generated accessors do not clash with the non-nullable variants.

Now possible:

locations.filter {
    (gps.latitude ?: Double.MIN_VALUE) >= 0.0
}

Also fixing #176. might need some more convertTo tests, but the example from the issue now works.

@Jolanrensen Jolanrensen changed the title Nullable accessors Nullable accessors and fixed convertTo Oct 5, 2022
@koperagen
Copy link
Collaborator

let's use rebase instead of merge to pull changes from dev branch in the future. i think that would make 1 less commit in the history

override val fieldType: FieldType = this@toNullable.fieldType.toNullable()
}

public fun BaseField.toNotNullable(): BaseField =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe, I'm not sure. I needed toNullable, but for completeness if we need it in the future I thought it best to also create a reverse option. Might save time to figure out how it works again

@Jolanrensen Jolanrensen merged commit c56df1e into master Oct 10, 2022
@Jolanrensen Jolanrensen deleted the nullable-accessors branch October 10, 2022 13:15
@Jolanrensen Jolanrensen added this to the 0.9.0 milestone Dec 12, 2022
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