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

literalinclude directives #239

Merged
merged 4 commits into from
May 20, 2024
Merged

literalinclude directives #239

merged 4 commits into from
May 20, 2024

Conversation

ucodery
Copy link
Collaborator

@ucodery ucodery commented Apr 10, 2024

This is my proof-of-concept of replacing most code blocks with linteralincludes from separate files maintained in the VCS. It is not meant to be merged as-is but to inspire discussion with a live example.

The idea is that, rather than type out example code every time, code that is often nearly or entirely identical, this code can be written out once and reused across the documentation.

Pros:

  • examples can't get out of sync
  • readers see the same example code everywhere, maintaining consistency
  • the example files can be fully linted/tested as part of an overall working project structure

Unintentionally, just moving this file's examples to individual files revealed two syntax errors at pyproject-toml-python-package-metadata.md:{200,209}.

Cons:

  • an extra layer of abstraction, possible barrier to new contributors, have to open 2 or more files for the same update
  • changes to the example code can cause cascading docs failures (examples might still pass all tests, but editors of examples must know what every window into these files expects to see)

@lwasser
Copy link
Member

lwasser commented May 6, 2024

@ucodery this is really cool. can you tell me how this works in terms of checking the pyproject.toml file?

you wrote this:

Unintentionally, just moving this file's examples to individual files revealed two syntax errors at pyproject-toml-python-package-metadata.md:{200,209}.

is there some built in sphinx check? as this is a toml file?
that is actually a fairly significant benefit from my perspective. i've had inadvertent mistakes several times in sample pyproject.toml files that i have created (i always forget to add quotes around version = "" for instance).

we could potentially add notes for contributors about hit the includes work if that is a concern. but from my perspective this seems like a significant improvement.

@lwasser
Copy link
Member

lwasser commented May 6, 2024

does anyone else see a reason to NOT move to this type of workflow for example toml (and other files)? the files are also then downloadable (potentially?) by users.

Copy link
Member

@lwasser lwasser left a comment

Choose a reason for hiding this comment

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

i don't have any changes, Jeremy. this looks like a great improvement to our guidebook. we can document how it works. i think it's unlikely for a super new-to-packaging contributor to want to modify the .toml file

@ucodery
Copy link
Collaborator Author

ucodery commented May 7, 2024

is there some built in sphinx check? as this is a toml file?

Nothing like a build-time check. IIRC it was my IDE telling me I was writing bad toml when I copied over the file. But my plan would be to actually build and test this example project, necessitating correct syntax (at least) for all files. This would probably be a CI action, but separate from building the site.

If you like this I can add contributing documentation, then it could be ready for merge. And I would follow-on to extract most other code examples.

@lwasser
Copy link
Member

lwasser commented May 7, 2024

If you like this I can add contributing documentation, then it could be ready for merge. And I would follow-on to extract most other code examples.

Yes please. i think this sounds great. would you then create a small demo package? i'm only asking because we do have this repo already: https://github.com/pyOpenSci/pyosPackage but it's obviously a separate repo which wouldn't work with what you are suggesting here. My take on your approach is that it will ensure that we use valid examples! In the future we can even use nbsphinx or one of the other tools to run code in our guidebook (if we need to).

@ucodery
Copy link
Collaborator Author

ucodery commented May 9, 2024

Yes, there will be a full demo package maybe including code that never appears in the docs, just so that we can test such a project end-to-end - not only linting, but testing that it actually does what we say it does in the docs. For now I will probably create a separate "dummy" project very similar, if not identical, to pyosPackage. In the future we can perhaps sync them up better (e.g. some sort of CI keeping them in sync or git submodules). It also might be that we need to have several demo projects in the documentation repo (hatchling vs meson, flat vs nested layouts...) and I don't know that you want all that in one example project.

@lwasser
Copy link
Member

lwasser commented May 9, 2024

ahhh ok. this makes sense! the syncing is my only concern then so we're not maintaining another package you know? but if there is some way to manage that via ci or submodule that would be great

but i would in the future like to have an example project using something like meson that is non pure python. we just aren't quite there yet.

btw i also have this: https://github.com/pyOpenSci/examplePy 😆 where i was playing with a lot of the tools. but you can see i haven't touched it in a long time!

@ucodery ucodery changed the title PoC: literalinclude directives literalinclude directives May 10, 2024
@ucodery ucodery marked this pull request as ready for review May 10, 2024 16:53
@ucodery ucodery requested a review from lwasser May 10, 2024 16:53
@ucodery
Copy link
Collaborator Author

ucodery commented May 10, 2024

This branch is now ready to merge. I have opened #248 #249 and #250 for follow-on work.

@lwasser
Copy link
Member

lwasser commented May 10, 2024

@all-contributors please add @ucodery for code, review, maintenance, documentation

i am not sure how this happened @ucodery but somehow you have maintainer status here and are not on the list of contributors. let's fix this now!

Copy link
Contributor

@lwasser

I've put up a pull request to add @ucodery! 🎉

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated
Comment on lines 84 to 86
If you need a net new example and it doesn't fit into any existing example files, you can create a new file and
reference it in a `literalinclude`. If it makes sense for that file to live within one of the existing example
projects please add it there; otherwise create a new folder in the examples.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If you need a net new example and it doesn't fit into any existing example files, you can create a new file and
reference it in a `literalinclude`. If it makes sense for that file to live within one of the existing example
projects please add it there; otherwise create a new folder in the examples.
If you want to add a new example that doesn't fit into any of the existing example files, you can create a new file and
reference it in a `literalinclude` block. If it makes sense for that file to live within one of the existing example
projects please add it there; otherwise create a new folder in the examples directory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added this locally with other changes.

@ucodery ucodery force-pushed the literalinclude branch 2 times, most recently from b2560d6 to 9492fed Compare May 20, 2024 15:35
@ucodery ucodery requested a review from lwasser May 20, 2024 15:42
@ucodery ucodery merged commit a9fc8c1 into pyOpenSci:main May 20, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants