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

Backlogged doc updates #247

Merged
merged 6 commits into from
Apr 28, 2023
Merged

Backlogged doc updates #247

merged 6 commits into from
Apr 28, 2023

Conversation

2bndy5
Copy link
Collaborator

@2bndy5 2bndy5 commented Apr 27, 2023

Some docs updates that have accumulated.

Also (as part of the suggestions from #242 ) the hard-coded pytest CLI options have been moved from pyproject.toml to nox_file.py

resolves #162
resolves #190
resolves #234
resolves #242

@2bndy5 2bndy5 added the documentation Improvements or additions to documentation label Apr 27, 2023
@2bndy5 2bndy5 force-pushed the backlogged-doc-updates branch from e389078 to de31364 Compare April 27, 2023 09:15
move the pytest hardcoded options from pyproject.toml to noxfile.py

adds a tests/README.rst to document the dev workflow for unit testing.

fix typo in index.rst


more proofreading
@2bndy5 2bndy5 force-pushed the backlogged-doc-updates branch from de31364 to 0f86af1 Compare April 27, 2023 09:32

npm install
npm run build
pip install -e .
Copy link
Owner

Choose a reason for hiding this comment

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

I believe pip install -e also does the npm build automatically. Is that not correct?

Copy link
Collaborator Author

@2bndy5 2bndy5 Apr 27, 2023

Choose a reason for hiding this comment

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

No, we had someone ask recently why sphinx failed to find the static bundles when it was using an editable install.

#238 (comment)

Copy link
Owner

Choose a reason for hiding this comment

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

Looks like the issue is that pip install -e . no longer runs the develop command, which is what we were relying on in setup.py to do the nodejs build. It looks like there is a new mechanism that can be used to handle editable builds, but it is not clear to me exactly how to use it:

https://setuptools.pypa.io/en/latest/userguide/extension.html#supporting-sdists-and-editable-installs-in-build-sub-commands

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 honestly don't mind the way it works now. It cuts down on the initial editable install time significantly.

Although, I do understand the convenience for newcomers not having to remember the npm build step. Should this become a separate issue?

Copy link
Owner

Choose a reason for hiding this comment

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

I created an issue. You don't need to re-run pip install -e . that often -- that is, after all, the whole point of an editable install. So I think it is definitely desirable for it to just work by default.

@jbms
Copy link
Owner

jbms commented Apr 27, 2023

Thanks!

also duplicate annotation for a repeated code annotation
@2bndy5
Copy link
Collaborator Author

2bndy5 commented Apr 27, 2023

I added a MyST example doc to satisfy #162. The MyST example doc is a bit raw, but it is meant to
encourage further contributions by providing a foundation to build on. Apparently, the blocking factor to user contribution is that the original doc uses a private API (in myst_parser._docs module) to behave similarly to our rst-example directive. However, the myst-example directive relies on a div directive that is hidden elsewhere in the MyST ecosystem (probably not meant to be used publicly as well).

@2bndy5 2bndy5 force-pushed the backlogged-doc-updates branch from 8f71de4 to c62da57 Compare April 28, 2023 00:12
The MyST example doc is a bit raw, but it is meant to
encourage further contributions by providing a foundation to build on.
@2bndy5 2bndy5 force-pushed the backlogged-doc-updates branch from c62da57 to ff1fc6e Compare April 28, 2023 00:50
@jbms
Copy link
Owner

jbms commented Apr 28, 2023

I added a MyST example doc to satisfy #162. The MyST example doc is a bit raw, but it is meant to encourage further contributions by providing a foundation to build on. Apparently, the blocking factor to user contribution is that the original doc uses a private API (in myst_parser._docs module) to behave similarly to our rst-example directive. However, the myst-example directive relies on a div directive that is hidden elsewhere in the MyST ecosystem (probably not meant to be used publicly as well).

I think our existing rst-example directive could easily be modified to support myst as well. In fact I think the only thing that needs to change is the language used for syntax highlighting, because when used from myst the nested_parse call will already do the right thing. (I think the div directive used by the myst-example directive defined by myst-parser comes from sphinx-design.)

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Apr 28, 2023

I think our existing rst-example directive could easily be modified to support myst as well.

I had the same thought a while ago: #162 (comment). But that was more geared toward converting rST to MyST on-the-fly and rendering both with the appropriate parsers.

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Apr 28, 2023

I think the myst-example directive is more of a long-term and worthwhile goal. The solution here is only meant to satisfy #162 per user's request/feedback. I'm really just trying to close out as many docs-related issues with this PR.

@jbms
Copy link
Owner

jbms commented Apr 28, 2023

I think the myst-example directive is more of a long-term and worthwhile goal. The solution here is only meant to satisfy #162 per user's request/feedback. I'm really just trying to close out as many docs-related issues with this PR.

Okay, sounds reasonable.

@2bndy5 2bndy5 merged commit 5d4cdf7 into main Apr 28, 2023
@2bndy5 2bndy5 deleted the backlogged-doc-updates branch April 28, 2023 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
2 participants