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

KDocs start using Doc Preprocessor Gradle plugin #214

Merged
merged 55 commits into from
Mar 27, 2023
Merged

Conversation

Jolanrensen
Copy link
Collaborator

@Jolanrensen Jolanrensen commented Dec 28, 2022

The final version of this PR consists of:

  • the ColumnExpression alias and use of it across the API instead of a specific Selector.
  • General concept docs with reusable examples in the :core documentation package
  • Docs for update.kt
  • Docs for Nulls.kt

The rendered version of the docs can be found at :core/generated-sources and it would probably be useful to review those as well as the source files. I'm curious to hear about how you find the resulting docs and whether the structure I provided for the two API files (and the common docs) is repeatable for the rest of the API or if you have some additions, concerns etc.

Old message:

Exploratory method for being able to reuse KDocs.
After trying, it seems that compiler plugins won't do the trick, since the javadoc and sources.jar are generated from the actual sources before the plugin is run.
So, to have the sources.jar be modified, we need something to run from Gradle itself.
Cue JCP. I've used this in the Kotlin Spark API as well.

The current implementation takes the sources from :core, runs the preprocessor on them, changes the Kotlin compile sources to the files modified by the preprocessor, and after compilation resets them back. With this method, sources.jar will contain the properly preprocessed files.
One downside is that compile errors will thus point you to the preprocessor files.

Since we (probably) will only use JCP to generate KDocs I will experiment and see if there's a way to only change the sources when generating sources.jar and not during compilation.

I've also filed an issue for multiline string variables for JCP since defining reusable comments in the same file now requires them to be a single line... It is possible to define the comments in the Gradle build files, but this doesn't seem right to me.

Check add.kt for an example.

Edit: JCP output is now only used for Jar tasks :D

@Jolanrensen Jolanrensen added documentation Improvements or additions to documentation (not KDocs) enhancement New feature or request labels Dec 28, 2022
@Jolanrensen Jolanrensen added this to the 0.10.0 milestone Dec 28, 2022
@Jolanrensen
Copy link
Collaborator Author

The only problem atm is in usability. Due to the lack of multiline strings, we need to do:

import org.jetbrains.kotlinx.dataframe.DataFrame
import org.jetbrains.kotlinx.dataframe.exceptions.DuplicateColumnNamesException
import org.jetbrains.kotlinx.dataframe.exceptions.UnequalColumnSizesException

/** either
//#local ADD = evalFile("add.txt")
*/

/** or
//#local ADD0 = " * Original [DataFrame] is not modified."
//#local ADD1 = " *"
//#local ADD2 = " * @throws [DuplicateColumnNamesException] if columns in expected result have repeated names"
//#local ADD3 = " * @throws [UnequalColumnSizesException] if columns in expected result have different sizes"
//#local ADD4 = " * @return new [DataFrame] with added columns"
//
//#local ADD = ADD0 + "\n" + ADD1 + "\n" + ADD2 + "\n" + ADD3 + "\n" + ADD4
 */


/**
 * Creates new [DataFrame] with given columns added to the end of original [DataFrame.columns] list.
 *
/*$ADD$*/
 * @param columns columns to add
 */
public fun <T> DataFrame<T>.add(vararg columns: AnyBaseCol): DataFrame<T> = addAll(columns.asIterable())

@Jolanrensen
Copy link
Collaborator Author

Using my self-built gradle plugin is probably a better solution https://github.com/Jolanrensen/kdocIncludeGradlePlugin

@Jolanrensen Jolanrensen changed the title JCP KDoc reuse [Exploratory] KDoc reuse Jan 9, 2023
@Jolanrensen
Copy link
Collaborator Author

Updated to use my own gradle plugin now via JitPack :)

Example can be found in add.kt:
image

@Jolanrensen
Copy link
Collaborator Author

https://github.com/Jolanrensen/docProcessorGradlePlugin/tree/main was updated to allow other processors as well. Maybe @sample is also interesting for DataFrame :)

@Jolanrensen Jolanrensen added KDocs Improvements or additions to KDocs and removed documentation Improvements or additions to documentation (not KDocs) labels Jan 24, 2023
@Jolanrensen Jolanrensen changed the title [Exploratory] KDoc reuse KDocs start using Doc Preprocessor Gradle plugin Mar 14, 2023
@Jolanrensen Jolanrensen marked this pull request as draft March 21, 2023 11:48
@zaleslaw
Copy link
Collaborator

What is the future of this PR?

@Jolanrensen
Copy link
Collaborator Author

What is the future of this PR?

Will rebase on master, fix every conflict and then ask for a review :)

# Conflicts:
#	core/build.gradle.kts
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/aliases.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/ColumnsSelectionDsl.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/Nulls.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/add.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/gather.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/reorder.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/select.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/update.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/convertTo.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/gather.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/reorder.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/update.kt
#	core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/columns/constructors.kt
@Jolanrensen Jolanrensen marked this pull request as ready for review March 23, 2023 18:02
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file I just started with, will probably work on docs after this PR is merged in a separate branch

@Jolanrensen
Copy link
Collaborator Author

ready for review!

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.

Not so many issues in reality, I will try to summarize them

  • be accurate with inserted code and probably wrap with the kotlin code insertion
  • convert some todos into issues or fix them
  • answer some questions
  • if possible, extract abstraction required for documentation building to the separate places

Some notes for the future:

  • Did we have a chance to use constants instead of interfaces?
  • Could we use @see tag instead of direct links with pattern?
  • Could we reuse the same snipppets of code used in the documentation to insert them and checking compiling it once

@Jolanrensen
Copy link
Collaborator Author

Not so many issues in reality, I will try to summarize them

  • be accurate with inserted code and probably wrap with the kotlin code insertion
  • convert some todos into issues or fix them
  • answer some questions
  • if possible, extract abstraction required for documentation building to the separate places

Some notes for the future:

  • Did we have a chance to use constants instead of interfaces?
  • Could we use @see tag instead of direct links with pattern?
  • Could we reuse the same snipppets of code used in the documentation to insert them and checking compiling it once

I cannot wrap code samples with triple backticks because then links inside won't be clickable anymore. If that was what you meant with be accurate with inserted code and probably wrap with the kotlin code insertion.

if possible, extract abstraction required for documentation building to the separate places
Do you suggest moving general docs from, say update.kt, to a separate file? say updateDocs.kt? I was debating myself whether to do that or not, because it would clutter the internal namespace more as well as moving the information about a file away from the file. I'd rather have them close together, I think. That said, the API is less clear if it's filled with docs.

Did we have a chance to use constants instead of interfaces?
Yes, constants are possible, but unlike interfaces they cannot be nested. Since the interfaces are never instantiated I don't think there's a speed loss. Isn't unreachable/unused code removed from the bytecode?

Could we use @see tag instead of direct links with pattern?
yes, but currently @see tags only show up rendered at the bottom of the doc with no description. That's how I came to the {@include [SomethingLink]} pattern, where interface SomethingLink contains the proper name and destination for what you're pointing to.

Could we reuse the same snipppets of code used in the documentation to insert them and checking compiling it once
We cannot check compilation from the Doc processor. It just processes docs ;). But with @sample you can include the same snippets from the documentation (if they are in the same module, I believe).

@zaleslaw
Copy link
Collaborator

Do you suggest moving general docs from, say update.kt, to a separate file? say updateDocs.kt? I was debating myself whether to do that or not, because it would clutter the internal namespace more as well as moving the information about a file away from the file. I'd rather have them close together, I think. That said, the API is less clear if it's filled with docs.

Probably, I'll try to do it in my PR with docs and we could compare two approaches

@Jolanrensen Jolanrensen merged commit 6c6a26c into master Mar 27, 2023
@Jolanrensen Jolanrensen deleted the jpc-kdoc-reuse branch March 27, 2023 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request KDocs Improvements or additions to KDocs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants