Skip to content

Conversation

@AndreiKingsley
Copy link
Collaborator

@AndreiKingsley AndreiKingsley commented Nov 11, 2025

Helps #527 - adds missing Path overloads

public fun DataFrame.Companion.read(path: Path, header: List<String> = emptyList()): AnyFrame =
read(
path = path,
format = guessFormat(path.toString())?.also {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't use guessFormatForExtension() anymore, does it? the logic has changed a bit


class GuessOverloadsTest {

private fun sampleJson(): String =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can do with a @Language("JSON") :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

also, const val

""".trimIndent()

@Test
fun read_guess_overloads_String_Path_File_produce_same_df() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is this snake_case btw? XD usually we write test names with backticks and spaces

Copy link
Collaborator

Choose a reason for hiding this comment

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

Junie, is this you? ;P

Copy link
Collaborator

Choose a reason for hiding this comment

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

You need proper junie guide))

class HtmlOverloadsTest {

@Test
fun writeHtml_overloads_String_Path_File_produce_same_content() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

html.writeHtml(fileOut)
html.writeHtml(strOut)

fun readText(p: Path): String = String(Files.readAllBytes(p), Charsets.UTF_8)
Copy link
Collaborator

Choose a reason for hiding this comment

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

uhm, you can just call path.readText() in Kotlin. This function is useless

import kotlin.io.path.createTempDirectory
import kotlin.io.path.writeText

class CsvOverloadsTest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this doesn't add much value


@Test
fun writeExcel_overloads_String_Path_File_create_nonempty_files() {
val df = dataFrameOf("name", "age")(
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can @Suppress("ktlint:standard:argument-list-wrapping") to make it look better. Or use the "name" to columnOf() api

fun GeoDataFrame<*>.writeGeoJson(path: Path) {
val featureJSON = FeatureJSON()
file.outputStream().use { outputStream ->
Files.newOutputStream(path).use { outputStream ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not use the kotlin api XD path.outputStream()

directory.mkdirs()
/** Path overload for writing Shapefile to a directory */
fun GeoDataFrame<*>.writeShapefile(directory: Path) {
if (!Files.exists(directory)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

also this has Kotlin extension functions, similar to File


return isOpenApiStr(file.readText())
public fun isOpenApi(path: Path): Boolean {
val name = path.fileName?.toString()?.lowercase() ?: return false
Copy link
Collaborator

Choose a reason for hiding this comment

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

path also has .extension, just like files

val yaml = tmp.resolve("api.yml")
yaml.writeText("openapi: 3.0.0\ncomponents:\n schemas: {}\n")
val json = tmp.resolve("api.json")
json.writeText("{" + "\"openapi\":\"3.0.0\",\"components\":{\"schemas\":{}}}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's this +?

Copy link
Collaborator

@zaleslaw zaleslaw left a comment

Choose a reason for hiding this comment

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

  1. Naming in tests
  2. @BeforeClass if it's possible
  3. Clear the point about deleting files with Comparator
  4. Clear the code from unexpected stuff


class JsonOverloadsTest {

private fun sampleJson(): String =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Create @before or @BeforeAll and move initialization there
https://www.baeldung.com/junit-before-beforeclass-beforeeach-beforeall

Why it's important?

  • better structured
  • performant (run once before all tests, not for each test)
  • best practice for Kotlin

writeJson(file.toPath(), prettyPrint)
}

public fun AnyFrame.writeJson(path: Path, prettyPrint: Boolean = false) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's improve here! No public method without KDoc

}
}

fun GeoDataFrame<*>.writeShapefile(directory: File) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please say, are these write functions part of public API or not? I guess it's a public and should be with apropriate modifier and KDocs?

}

fun GeoDataFrame<*>.writeGeoJson(file: File) {
// TODO: adds ids that breaks order of reading
Copy link
Collaborator

Choose a reason for hiding this comment

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

TODO: issue or it will be forgotten


class CsvOverloadsTest {

private fun sampleCsv(): String =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks weird to me, better to prepare test data in @BeforeClass section and reuse it

compression = compression,
adjustCsvSpecs = ADJUST_CSV_SPECS,
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, but from this edits with +/- is not clear, did you removed a adjustCsvSpecs = ADJUST_CSV_SPECS parameter? and what's happened with compression = Compression.of(file.toPath()),?

r.rowsCount() shouldBe df.rowsCount()
r.columnNames() shouldBe df.columnNames()
} finally {
Files.walk(tmp).sorted(Comparator.reverseOrder()).forEach { it.toFile().delete() }
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's happened if it will be an exception in finally block? on File.delete()
Do we really need to clear the tmp directory in this way?

}

@Test
fun ipc_overloads_write_and_read() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename testname to back stick

Copy link
Collaborator

Choose a reason for hiding this comment

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

*backtick XD

try {
val file = tmp.resolve("schema.json").toFile().also { it.writeText("{}") }
val path = file.toPath()
val str = path.toUri().toString() // String-оверлоад ожидает URL/URI-строку
Copy link
Collaborator

@zaleslaw zaleslaw Nov 12, 2025

Choose a reason for hiding this comment

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

Das ist nicht auf Russisch geschrieben, aber auf eine translatenend und machine-generiert subset von Russisch!

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.

5 participants