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

First version #1

Merged
merged 13 commits into from
May 13, 2023
Merged

First version #1

merged 13 commits into from
May 13, 2023

Conversation

binste
Copy link
Contributor

@binste binste commented May 13, 2023

See vega/altair#3051 for context.

For the review:

  • altairplot.py: I copied it over from Altair in the first commit and added some type hints and did smaller docstring improvements in subsequent commits. I therefore think this does not require a detailed review.
  • Package management related files, README, etc.: I tried to keep these as close as possible to the altair repo
  • tests: Tests are inspired by some other sphinx extensions and by the sphinx repo itself which uses the same folder structure and approach to test directives.

The README.md has some instructions on how this package can be manually tested if required by building the test documentation. Furthermore, I created a PR over in the altair repo vega/altair#3056 which uses the code from this branch to build the whole altair documentation. That worked well without any issues.

Not sure why workflows are not yet executed, do they need to be merged first at least once? They are executed in my fork so you can view the status there: https://github.com/binste/sphinxext-altair/actions

Happy to do a release on pypi once this gets approved and add @mattijn and @joelostblom as maintainers as well. I can of course also add more if you want.

@binste binste requested a review from mattijn May 13, 2023 17:30
@binste binste marked this pull request as ready for review May 13, 2023 17:30
pyproject.toml Outdated Show resolved Hide resolved
@binste binste merged commit 39754b0 into vega:main May 13, 2023
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.

2 participants