Skip to content

Build config and Debug mode #796

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 3 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ so do familiarize yourself with the following guidelines.

## PR workflow

0. The contributor builds the library locally and runs all unit tests via the Gradle task `dataframe:test`
(see the ["Building"](#building) chapter).
0. The contributor builds the library locally and runs all unit tests via the Gradle task
`dataframe:test -Pkotlin.dataframe.debug=true` (see the ["Building"](#building) chapter).
1. The contributor submits the PR if the local build is successful and the tests are green.
2. The reviewer puts their name in the "Reviewers" section of the proposed PR at the start of the review process.
3. The reviewer leaves comments or marks the PR with the abbreviation "LGTM" (Looks good to me).
Expand Down Expand Up @@ -103,6 +103,8 @@ This library is built with Gradle.
* Run `./gradlew build` to build. It also runs all the tests and checks the linter.
* Run `./gradlew <module>:test` to test the module you are looking at to speed
things up during development.
* Make sure to pass the extra parameter `-Pkotlin.dataframe.debug=true` to enable debug mode. This flag will
make sure some extra checks are run, which are important but too heavy for production.

You can import this project into IDEA, but you have to delegate the build actions
to Gradle (in Preferences -> Build, Execution, Deployment -> Build Tools -> Gradle -> Runner)
Expand Down
14 changes: 14 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import com.github.benmanes.gradle.versions.updates.DependencyUpdatesTask
import com.github.gmazzo.buildconfig.BuildConfigExtension
import org.jetbrains.kotlin.gradle.dsl.KotlinJvmProjectExtension
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
import org.jetbrains.kotlinx.dataframe.AnyFrame
Expand All @@ -25,6 +26,7 @@ plugins {
alias(docProcessor) apply false
alias(simpleGit) apply false
alias(dependencyVersions)
alias(buildconfig) apply false

// dependence on our own plugin
alias(dataframe) apply false
Expand Down Expand Up @@ -154,6 +156,18 @@ allprojects {

// set the java toolchain version to 11 for all subprojects for CI stability
extensions.findByType<KotlinJvmProjectExtension>()?.jvmToolchain(11)

// Attempts to configure buildConfig for each sub-project that uses it
try {
configure<BuildConfigExtension> {
packageName = "org.jetbrains.kotlinx.dataframe"
className = "BuildConfig"
buildConfigField("VERSION", "${project.version}")
buildConfigField("DEBUG", findProperty("kotlin.dataframe.debug")?.toString()?.toBoolean() ?: false)
}
} catch (_: UnknownDomainObjectException) {
logger.warn("Could not set buildConfig on :${this.name}")
}
}
}

Expand Down
1 change: 1 addition & 0 deletions core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ plugins {
alias(ktlint)
alias(docProcessor)
alias(simpleGit)
alias(buildconfig)

// dependence on our own plugin
alias(dataframe)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
package org.jetbrains.kotlinx.dataframe.impl.columns

import org.jetbrains.kotlinx.dataframe.BuildConfig
import org.jetbrains.kotlinx.dataframe.DataColumn
import org.jetbrains.kotlinx.dataframe.api.dataFrameOf
import org.jetbrains.kotlinx.dataframe.impl.isArray
import org.jetbrains.kotlinx.dataframe.impl.isPrimitiveArray
import kotlin.reflect.KClass
import kotlin.reflect.KType
import kotlin.reflect.full.isSubclassOf

internal abstract class DataColumnImpl<T>(
protected val values: List<T>,
Expand All @@ -12,6 +17,31 @@ internal abstract class DataColumnImpl<T>(
) : DataColumn<T>,
DataColumnInternal<T> {

private infix fun <T> T?.matches(type: KType) =
when {
this == null -> type.isMarkedNullable

this.isPrimitiveArray ->
type.isPrimitiveArray &&
this!!::class.qualifiedName == type.classifier?.let { (it as KClass<*>).qualifiedName }

this.isArray -> type.isArray

// cannot check the precise type of array
else -> this!!::class.isSubclassOf(type.classifier as KClass<*>)
}

init {
// Check for [Issue #713](https://github.com/Kotlin/dataframe/issues/713).
// This only runs with `kotlin.dataframe.debug=true` in gradle.properties.
if (BuildConfig.DEBUG) {
require(values.all { it matches type }) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It checks all values, right? Is it worth to check random 10-20 numbers from the column?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we only check a handful of values there's a chance we won't catch the outlier and get flaky tests.

val types = values.map { if (it == null) "Nothing?" else it!!::class.simpleName }.distinct()
"Values of column '$name' have types '$types' which are not compatible given with column type '$type'"
}
}
}

protected val distinct = distinct ?: lazy { values.toSet() }

override fun name() = name
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package org.jetbrains.kotlinx.dataframe.jupyter

import org.jetbrains.kotlinx.dataframe.BuildConfig
import org.jetbrains.kotlinx.dataframe.io.DisplayConfiguration

public class JupyterConfiguration {
public val display: DisplayConfiguration = DisplayConfiguration()

/** Version of the library. */
public val version: String = BuildConfig.VERSION

/** DSL accessor. */
public operator fun invoke(block: JupyterConfiguration.() -> Unit): JupyterConfiguration = apply(block)
}
5 changes: 5 additions & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,8 @@ org.gradle.jvmargs=-Xmx4G
# This makes it mandatory to explicitly apply your own version of the
# KSP plugin in the modules that use it.
kotlin.dataframe.add.ksp=false

# Enables debug mode for dataframe.
# This can make certain tests run that should not be run in production.
# It can also be turned on from the command line with `-Pkotlin.dataframe.debug=true`
kotlin.dataframe.debug=false
Copy link
Collaborator

Choose a reason for hiding this comment

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

does it mean, that we could add more modes, with different names, it's not a strongly-defined schema?

Copy link
Collaborator Author

@Jolanrensen Jolanrensen Jul 30, 2024

Choose a reason for hiding this comment

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

indeed, just an argument I came up with. Similar to the add.ksp argument.
It's not strongly typed, but when read it tries to parse the argument as Boolean and if failed it defaults to false

2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ android-gradle-api = "7.3.1" # Can't be updated to 7.4.0+ due to Java 8 compatib
ktor-server-netty = "2.3.8"
kotlin-compile-testing = "1.6.0"
duckdb = "0.10.0"
buildconfig = "5.4.0"

[libraries]
ksp-gradle = { group = "com.google.devtools.ksp", name = "symbol-processing-gradle-plugin", version.ref = "ksp" }
Expand Down Expand Up @@ -149,3 +150,4 @@ kover = { id = "org.jetbrains.kotlinx.kover", version.ref = "kover" }
dependencyVersions = { id = "com.github.ben-manes.versions", version.ref = "dependencyVersions" }
plugin-publish = { id = "com.gradle.plugin-publish", version.ref = "plugin-publish" }
shadow = { id = "com.github.johnrengelman.shadow", version.ref = "shadow" }
buildconfig = { id = "com.github.gmazzo.buildconfig", version.ref = "buildconfig" }
Loading