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

fix: Apply schema.json normalization, add to docs #1265

Merged
merged 10 commits into from
Apr 25, 2024

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Apr 23, 2024

references

changes

  • add some additional constraints to pydantic model
  • add $id and $schema to generated schema.json
  • update some prose in pydantic model
  • apply ordering normalization to generated schema.json
    • remove all references to null (not representable in either TOML file)
  • deploy schema.json to the docs site (under schema/manifest/schema.json)
  • use a single scope="session" fixture for the schema validation tests (anecdotally cuts time in about half)

@bollwyvl bollwyvl changed the title Apply schema.json normalization, add to docs fix: Apply schema.json normalization, add to docs Apr 23, 2024
@bollwyvl bollwyvl marked this pull request as ready for review April 23, 2024 17:11
@bollwyvl
Copy link
Contributor Author

Welp, I don't know enough serde to add SchemaURI or whatever it should be called to TomlProjectManifest, but otherwise this (starts) ticking all the boxes from #1258.

There are still a bunch of places where the pydantic model kinda falls down for in-IDE inspectability, e.g. human-readable title/description on an anyOf defined with |... just takes a bunch more Annotated[x | y, Field(description)] to do properly, but would probably be beneficial.

A longer docs con (in addition to the raw schema.json) would be to use the schema as the source-of-truth for the docs, but I haven't explored the options for doing that directly in mkdocs, or any of the myriad generators.

Even more value can be squeezed from the schema with e.g typify, but again, I don't sling enough rust to distinguish various body parts. But at that point, even having pydantic in the loop might be less effective than writing the schema by hand in e.g. TOML so that whitespace and rst hangover, etc. becomes less of an issue.

@@ -132,6 +132,9 @@ nav:
- Community: Community.md
- FAQ: FAQ.md

hooks:
Copy link
Member

Choose a reason for hiding this comment

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

great. we should use this for moving the install.sh script as well cc @ruben-arts :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed: my first foray into working the mkdocs API. Not really worse than sphinx, but, ironically, much harder-to-read docs even though the API seems a little more sane/generalized.

@bollwyvl
Copy link
Contributor Author

Gah, somehow posting that comment reloaded an older version of this post, so lost some description stuff. No use crying about it, I suppose.

Happy for any further feedback (or drive-by rust coding): no further changes planned from my side.

@ruben-arts
Copy link
Contributor

Hey this looks really good!
I have one question, how do I use that $schema in the pixi.toml? I think it would be good to added it to one of the examples for testing. (or in the actual schema tests). I can't get it to work myself in vscode or Jetbrains

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Apr 25, 2024

how do I use that

The most full-featured TOML tool that supports $schema is the rust-based taplo, which is available on conda-forge or npm (as WASM). There's an in-flight PR to build it for PyPI.

Once on PyPI, I'll likely work to get it added to the "well-known" specs on jupyter-lsp as well, but could be configured already by JSON.

At the command line, it can do things like:

> taplo format pixi.toml
INFO taplo:lint_files:collect_files: found files total=1 excluded=0 cwd="/home/weg/projects/pixi_/pixi"

(which would, with the defaults, yield):

diff --git a/pixi.toml b/pixi.toml
index 8df6b59..232b5e9 100644
--- a/pixi.toml
+++ b/pixi.toml
@@ -1,11 +1,13 @@
+"$schema" = "./schema/schema.json"
+
 [project]
 name = "pixi"
 description = "Package management made easy!"
 authors = [
-    "Wolf Vollprecht <wolf@prefix.dev>",
-    "Bas Zalmstra <bas@prefix.dev>",
-    "Tim de Jager <tim@prefix.dev>",
-    "Ruben Arts <ruben@prefix.dev>",
+  "Wolf Vollprecht <wolf@prefix.dev>",
+  "Bas Zalmstra <bas@prefix.dev>",
+  "Tim de Jager <tim@prefix.dev>",
+  "Ruben Arts <ruben@prefix.dev>",
 ]
 channels = ["conda-forge"]
 platforms = ["linux-64", "win-64", "osx-64", "osx-arm64"]

good job!

After adding an icky [foo]

> taplo lint pixi.toml 

 INFO taplo:lint_files:collect_files: found files total=1 excluded=0 cwd="/home/weg/projects/pixi_/pixi"
error: Additional properties are not allowed ('foo' was unexpected)
   ┌─ pixi.toml:1:1
   │  
 1 │ ╭ "$schema" = "./schema/schema.json"
 2 │ │ 
 3 │ │ [foo]
 4 │ │ 
   · │
70 │ │ schema = ["schema"]
71 │ │ 
   │ ╰^ Additional properties are not allowed ('foo' was unexpected)

vscode

The extension I've been using is:

Name: Even Better TOML
Id: tamasfe.even-better-toml
Description: Fully-featured TOML support
Version: 0.19.2
Publisher: tamasfe
VS Marketplace Link: https://marketplace.visualstudio.com/items?itemName=tamasfe.even-better-toml

This will use a taplo on $PATH when it is launched, or go grab the WASM one (buh).

image

jetbrains

🤷 ?

added it to one of the examples for testing.

Two of the testing examples already use the taplo-specific #schema: directive:

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Apr 25, 2024

I guess some ways forward (not mutually exclusive):

  • add taplo to [feature.schema.dependencies]
    • in addition to the (now somewhat faster) python jsonschema test, it could also run taplo lint
  • add $schema to the root pixi.toml
  • add taplo to pre-commit (not my favorite tool)
    • there's this thing
    • apparently, getting it added to pre-commit (not my favorite) is part of the PyPI motivation
  • add to the IDE integration docs
    • though somebody else would have to generate the 10 screenshots to get to the JetBrains one

Really up to you, happy to do some of the above, but will be out of pocket for a few days after tomorrow.

@bollwyvl
Copy link
Contributor Author

Longer con, and outside my wheelhouse: using $schema to suggest things at the CLI like:

> pixi info

# ...

Project
------------
     Manifest file: ~/pixi/pixi.toml
  Config locations: 
      Last updated: 22-04-2024 15:25:27
            Schema: https://pixi.sh/v0.19.0/schema/manifest/schema.json

And theoretically eventually something deeper, which actually used knowledge about the changes between the schema versions to suggest that things have changed, either baked into the binary, or stored in the schema (ick):

> pixi check

Project Schema: 0.19.0 

  You have pixi 0.20.0 installed which supports new schema features:

  [tasks.*.env]
    We now support `env` variables in the `task` definition, these can also be used as default values 
    for parameters in your task which you can overwrite with your shell's env variables.

        task = { 
          cmd = "task to run", 
          env = { VAR="value1", PATH="my/path:$PATH" } 
        }

@bollwyvl
Copy link
Contributor Author

taplo also supports the x-taplo schema extension. pydantic certainly doesn't know how to build those, so it would be very manual, but might improve the UX of an IDE running the taplo lsp language server. For example:

"$defs": {
  "Project" {
   // ...
   "x-taplo": {
     "initKeys": ["name", "channels"]
   }
}

When closing off a [project table header, the theory would be that it would create a name = "<default name>" and channels = [<default channels>].

I haven't spent any time actually trying to do that for any schema yet, and would see that as a follow-on activity.

@ruben-arts
Copy link
Contributor

Thanks @bollwyvl for the additional explanation, that sounds really useful! Though I think all of that can be added later. This is already a nice improvement. I tried adding the schema to the pixi.toml but we use pixi in the CI so maybe add it after the new release.

p.s. I love your write ups and conversations! Exceptionally high quality!

@ruben-arts ruben-arts enabled auto-merge (squash) April 25, 2024 20:24
@ruben-arts ruben-arts merged commit b7537d2 into prefix-dev:main Apr 25, 2024
28 checks passed
@bollwyvl
Copy link
Contributor Author

Thanks for the kind words and speedy review, sorry for any TZ-related delay in looking at stuff.

use pixi in the CI

This Is The Way. As is squash-merging my stream-of-consciousness commits. Thanks again!

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.

Improve pixi.toml schema syntax for human and machine consumption
3 participants