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

Make code blocks in src/vector/_compute/*.py parsable #240

Open
Saransh-cpp opened this issue Aug 17, 2022 · 9 comments · May be fixed by #551
Open

Make code blocks in src/vector/_compute/*.py parsable #240

Saransh-cpp opened this issue Aug 17, 2022 · 9 comments · May be fixed by #551
Labels
docs Improvements or additions to documentation good first issue Good for newcomers

Comments

@Saransh-cpp
Copy link
Member

Vector uses blacken-docs to prettify and format the docs. However, we use the -E flag to ignore any errors present in the code blocks.

Removing the -E option outputs the following errors -

blacken-docs.............................................................Failed
- hook id: blacken-docs
- exit code: 1

src/vector/_compute/lorentz/gamma.py:7: code block parse error Cannot parse: 2:0: Lorentz.gamma(self)
src/vector/_compute/lorentz/beta.py:7: code block parse error Cannot parse: 2:0: Lorentz.beta(self)
src/vector/_compute/lorentz/Et2.py:7: code block parse error Cannot parse: 2:0: Lorentz.Et2(self)
src/vector/_compute/spatial/eta.py:7: code block parse error Cannot parse: 2:0: Spatial.eta(self)
src/vector/_compute/planar/rho2.py:7: code block parse error Cannot parse: 2:0: Planar.rho2(self)
src/vector/_compute/planar/phi.py:7: code block parse error Cannot parse: 2:0: Planar.phi(self)
src/vector/_compute/spatial/cottheta.py:7: code block parse error Cannot parse: 2:0: Spatial.cottheta(self)
src/vector/_compute/lorentz/Mt.py:7: code block parse error Cannot parse: 2:0: Lorentz.Mt(self)
src/vector/_compute/lorentz/rapidity.py:7: code block parse error Cannot parse: 2:0: Lorentz.rapidity(self)
src/vector/_compute/spatial/z.py:7: code block parse error Cannot parse: 2:0: Spatial.z(self)
src/vector/_compute/lorentz/Et.py:7: code block parse error Cannot parse: 2:0: Lorentz.Et(self)
src/vector/_methods.py:497: code block parse error Cannot parse: 1:10: obj["xx"] obj["xy"]
src/vector/_methods.py:763: code block parse error Cannot parse: 1:10: obj["xx"] obj["xy"] obj["xz"]
src/vector/_methods.py:1080: code block parse error Cannot parse: 1:10: obj["xx"] obj["xy"] obj["xz"] obj["xt"]
src/vector/_compute/spatial/costheta.py:7: code block parse error Cannot parse: 2:0: Spatial.costheta(self)
src/vector/_compute/lorentz/boostX_gamma.py:7: code block parse error Cannot parse: 1:27: Lorentz.boostX(self, gamma=)
src/vector/_compute/planar/y.py:7: code block parse error Cannot parse: 2:0: Planar.y(self)
src/vector/_compute/spatial/mag2.py:7: code block parse error Cannot parse: 2:0: Spatial.mag2(self)
src/vector/_compute/planar/rho.py:7: code block parse error Cannot parse: 2:0: Planar.rho(self)
src/vector/_compute/lorentz/Mt2.py:7: code block parse error Cannot parse: 2:0: Lorentz.Mt2(self)
src/vector/_compute/lorentz/t2.py:7: code block parse error Cannot parse: 2:0: Lorentz.t2(self)
src/vector/_compute/lorentz/t.py:7: code block parse error Cannot parse: 2:0: Lorentz.t(self)
src/vector/_compute/lorentz/tau.py:7: code block parse error Cannot parse: 2:0: Lorentz.tau(self)
src/vector/_compute/lorentz/tau2.py:7: code block parse error Cannot parse: 2:0: Lorentz.tau2(self)
src/vector/_compute/spatial/mag.py:7: code block parse error Cannot parse: 2:0: Spatial.mag(self)
src/vector/_compute/spatial/theta.py:7: code block parse error Cannot parse: 2:0: Spatial.theta(self)
src/vector/_compute/planar/x.py:7: code block parse error Cannot parse: 2:0: Planar.x(self)

The hook -

- repo: https://github.com/asottile/blacken-docs
rev: v1.12.1
hooks:
- id: blacken-docs
args: ["-E"]
additional_dependencies: [black==22.3.0]

These code blocks should ideally be parsable, and the -E option should be removed

@Saransh-cpp Saransh-cpp added docs Improvements or additions to documentation good first issue Good for newcomers labels Aug 17, 2022
@Saransh-cpp Saransh-cpp added this to the v1.0.0 milestone Aug 29, 2022
@matthewfeickert
Copy link
Member

@Saransh-cpp were you thinking of things like simple examples to replace what Jim had added in PR #55?

--- a/src/vector/_compute/lorentz/tau2.py
+++ b/src/vector/_compute/lorentz/tau2.py
@@ -4,10 +4,11 @@
 # or https://github.com/scikit-hep/vector for details.
 
 """
-.. code-block:: python
+Example:
 
-    @property
-    Lorentz.tau2(self)
+    >>> import vector
+    >>> vector.obj(pt=3, phi=2, eta=1, mass=4).tau2
+    16.0
 """
 from __future__ import annotations

or something else that extends instead of replaces?

@Saransh-cpp
Copy link
Member Author

My initial thought was to replace the current examples with copy-pastable examples. Now that I think of it, the current examples are also very informative for vector's developers and users. Maybe we can do something like -

.. code-block:: python
 
    @property
    Lorentz.tau2(self)

+Example:
+
+.. code-block:: python
+
+    >>> import vector
+    >>> vector.obj(pt=3, phi=2, eta=1, mass=4).tau2
+    16.0

and let blacken-docs fail on the Lorentz.tau2(self) code block?

@Saransh-cpp Saransh-cpp modified the milestones: v0.11.0, v1.0.0 Dec 11, 2022
@Saransh-cpp Saransh-cpp removed this from the v1.0.0 milestone Feb 17, 2023
@Akhil-Sharma30
Copy link

Can I work on this issue?

@jpivarski
Copy link
Member

I think that would be very helpful. Thank you! @Saransh-cpp, could you help @Akhil-Sharma30 get started?

@henryiii
Copy link
Member

and let blacken-docs fail on the Lorentz.tau2(self) code block?

You don't need to "let it fail", just use pycon for the block language. Blacken-docs won't pick it up then (same with ipython and pytb), and the syntax highlighter might even do a better job (it does on GitHub, I know).

Or you can avoid the >>>, I'm not wildly fond of adding characters that can't be copy-pasted for copy-paste examples. (Unless the code renderer can make those parts not copy-passable, sometimes they can).

@Saransh-cpp
Copy link
Member Author

I think that would be very helpful. Thank you! @Saransh-cpp, could you help @Akhil-Sharma30 get started?

Yes, definitely! Sorry, I wasn't able to comment before.

@Akhil-Sharma30 can you start with @henryiii's suggestion and open a new PR for changing some of the code blocks to pycon? I think we can keep the code blocks as it is, given that they are developer-facing (the sub-package name starts with _), and not user-facing. So for example, you'll have to update -

.. code-block:: python

with pycon in the place of python. Similarly for all the other failing code blocks (you can remove "-E" from blacken-docs' config in .pre-commit-config.yaml to obtain all the errors locally.)

@Schefflera-Arboricola
Copy link
Contributor

Hi, I would like to work on this issue. But, I have a question-- why are we using .. code-block:: python in the developer facing docs? Because AFAICT these are not getting rendered and displayed on the vector's readthedocs.io website. So, wouldn't it make more sense to simply write these code blocks as multi-line comments, instead of replacing python with pycon or ipython or pytb (unless there are plans to show these in the user facing docs in the future).

So, my suggestion is:

"""
@property
Lorentz.tau2(self)
"""

Please LMK if my understanding is correct and if I should go ahead with the above approach. Thank you :)

@Schefflera-Arboricola Schefflera-Arboricola linked a pull request Dec 24, 2024 that will close this issue
7 tasks
@jpivarski
Copy link
Member

Before #545 (last week), all of the docstrings were being rendered in readthedocs, and they used ReST to do it. In that PR, I restricted the user-facing docs to just those that a user should be concerned about and anything that was rewritten was rewritten as Markdown, rather than ReST.

I did not remove docstrings from classes and functions that used to be visible in readthedocs and now aren't—there's a linter that requires non-underscored classes and functions to have docstrings, and anyway, why remove good documentation?

It no longer needs to be ReST-formatted because it's not being rendered in HTML, but I don't think there's any need to perform a campaign of removing ReST declarations. Since these are no longer rendered in readthedocs, they're explicitly expert-level documentation for developers going through the codebase, and experts can ignore some unnecessary ReST baggage.

Unless, of course, removing them helps to make all of the documentation adhere to a format enforced by black. In that case, if you're wondering whether the ReST declarations need to be preserved: they do not need to be preserved.

@Schefflera-Arboricola
Copy link
Contributor

Thank you for replying @jpivarski !

I kept the code-blocks for consistency, as removing them would require us to remove them in the non-property methods as well, resulting in a larger diff, which would be unnecessary, I think. Instead, I added a one-line note indicating that it’s a property method. This way it would also require less changes when/if examples get added to these docs, I think. LMKWYT. Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation good first issue Good for newcomers
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

6 participants