Build config and Debug mode #796
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
….version` using buildConfig
zaleslaw
reviewed
Jul 30, 2024
core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/columns/DataColumnImpl.kt
Outdated
Show resolved
Hide resolved
zaleslaw
reviewed
Jul 30, 2024
| * This only runs with `kotlin.dataframe.debug=true` in gradle.properties | ||
| */ | ||
| if (BuildConfig.DEBUG) { | ||
| require(values.all { it matches type }) { |
There was a problem hiding this comment.
It checks all values, right? Is it worth to check random 10-20 numbers from the column?
There was a problem hiding this comment.
If we only check a handful of values there's a chance we won't catch the outlier and get flaky tests.
zaleslaw
reviewed
Jul 30, 2024
| # 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 |
There was a problem hiding this comment.
does it mean, that we could add more modes, with different names, it's not a strongly-defined schema?
There was a problem hiding this comment.
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
zaleslaw
approved these changes
Jul 30, 2024
…enabled. Updated contribution guide too. TC will have to be updated manually
|
Thanks! |
|
Generated sources will be updated after merging this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the buildconfig plugin to the project.
This achieves two things:
BuildConfig.VERSION. We can use this well in the Jupyter integration where we're often interested which dataframe version is pulled into the notebook when doing a%use dataframe.type: KTypeinDataColumnImplmismatches actual values sometimes #713.By simply adding
-Pkotlin.dataframe.debug=trueto the build on TC, we can run the extra checks from the issue. By default this check should be off, as it costs performance, but it's very valuable for catches bugs. We can useBuildConfig.DEBUGin other places in the future as well.See each commit for more info.
We should add the flag to TC only after #713 is solved completely, otherwise all builds will fail.