-
Notifications
You must be signed in to change notification settings - Fork 113
Descriptors in libraries.json #488
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
Conversation
… anything a normal descriptor file can, like introduce dependencies, run init blocks etc. This allows libraries to adapt how they behave in notebooks without having to depend on the jupyter api
… in notebooks. Requires Kotlin/kotlin-jupyter#488
} | ||
|
||
@Suppress("unused") | ||
fun descriptor(descriptor: String) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't gradle have a better API for representing JSON's? BTW, probably use parameter lang injection here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return LibrariesScanResult( | ||
fqns("definitions"), | ||
fqns("producers"), | ||
definitions = fqns("definitions"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better remove everything related to FQNS_PATH: we don't scan annotations anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so removing getScanResultFromAnnotations() altogether?
.../shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/libraries/LibrariesScanner.kt
Outdated
Show resolved
Hide resolved
discardedFQNs.add(typeName) | ||
false | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant newline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ktlint adds it because of the {}
before it ;P
} | ||
} | ||
|
||
private fun Iterable<JsonObject>.filterDescriptorsToLoad() = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comment is welcomed here, would be nice to see a description of the trouble you experienced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, I can probably make this a bit more efficient by only storing the hash instead of the entire string
.../shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/libraries/LibrariesScanner.kt
Outdated
Show resolved
Hide resolved
jupyter-lib/shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/libraries/Parsing.kt
Outdated
Show resolved
Hide resolved
@Jolanrensen please file the MR in space, we have a safe merge there |
…gnored for future compatibility
Relates to Kotlin/dataframe#1140
Adds the ability to have full JSON library descriptors inside the libraries.json instead of just fqn's to integration classes.
This will be used in DataFrame to load the
dataframe-jupyter
artifact whendataframe-core
is present without it (which is common when using your project as dependency to the notebook).