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

nixpkgs manual, doc Python: render interpreters in a table #246956

Merged
merged 4 commits into from
Aug 7, 2023

Conversation

alejandrosame
Copy link
Contributor

The current paragraph presenting Python interpreters is verbose and hinders clarity. The information provided is well suited to be rendered as a table.

tag:alejandrosamemightyiam

Copy link
Member

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

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

That's a good idea. Found a few mistakes.

doc/languages-frameworks/python.section.md Outdated Show resolved Hide resolved
| Packages | Aliases | Interpeters |
|------------|-----------------|------------|
| python27 | python2, python | CPython 2.7 |
| python38 | ` ` | CPython 3.8 |
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
| python38 | ` ` | CPython 3.8 |
| python38 | | CPython 3.8 |

What do we need these backticks for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept trying to render the docs without them and they fail. The empty backticks worked as a workaround to be able to render empty cells.

What would be the proper way of getting empty cells? (Maybe it's just a bug that needs to be reported to nixos-render-docs)

Copy link
Member

Choose a reason for hiding this comment

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

Deferring to @pennae.

Copy link
Contributor

Choose a reason for hiding this comment

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

is indeed a bug. #246996

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the prompt fix! I'll wait for the merge to rebase and get rid of the backticks workaround.

Copy link
Contributor

Choose a reason for hiding this comment

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

could you test your changes with the nrd patch applied and report results? just in case you've uncovered any other bugs as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pennae After applying the patch (temp-branch), the whole table can be rendered without the empty code blocks workaround.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

awesome. fix is merged now.

doc/languages-frameworks/python.section.md Outdated Show resolved Hide resolved
@FRidh
Copy link
Member

FRidh commented Aug 3, 2023

Good idea, thanks!

compatible with Python 2.7 and 3 are available as `pypy27` and `pypy3`, with
aliases `pypy2` mapping to `pypy27` and `pypy` mapping to `pypy2`. The Nix
expressions for the interpreters can be found in
| Packages | Aliases | Interpreters |
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
| Packages | Aliases | Interpreters |
| Package | Aliases | Interpreter |

One package, has one interpreter, but potentially multiple aliases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point. Plurals can be misleading in table headers, and here I also see now that it's better not to use them to refer to the columns but per-row.

Header updated in latest commit.

@pennae pennae mentioned this pull request Aug 3, 2023
12 tasks
alejandrosame pushed a commit to mobusoperandi/nixpkgs that referenced this pull request Aug 7, 2023
`children` only has to be not-None, not also not-[]. empty table cells
would can produce an empty inline token and cause these checks to fail,
even though the token actually is totally valid.

showed up in
NixOS#246956 (comment).
pennae added a commit that referenced this pull request Aug 7, 2023
`children` only has to be not-None, not also not-[]. empty table cells
would can produce an empty inline token and cause these checks to fail,
even though the token actually is totally valid.

showed up in
#246956 (comment).
alejandrosame and others added 4 commits August 7, 2023 19:30
The current paragraph presenting Python interpreters is verbose and hinders clarity. The information provided is well suited to be rendered as a table.

Co-authored-by: Shahar "Dawn" Or <mightyiampresence@gmail.com>
@alejandrosame
Copy link
Contributor Author

@mweinelt with the nrd fix merged, all the feedback so far has been addressed. Are there any other changes required?

@mweinelt mweinelt merged commit c724801 into NixOS:master Aug 7, 2023
17 checks passed
@alejandrosame alejandrosame deleted the doc_python_interpreters branch August 8, 2023 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants