PEP 817: Final set of pre-split updates#4867
Conversation
This is a set of updates to PEP 817 that were prepared prior to the decision to split it into smaller PEPs. We do not intend to pursue PEP 817 in this updated form. However, we would like to get the changes committed to the repository so that the history of changes is not lost. The changes include refocusing of the security details, some clarifications, tool behavior outlines and update for the images to respect dark theme. We are not marking the PEP as withdrawn, as our current plan is to eventually replace the specification part with an index listing the split PEPs. Signed-off-by: Michał Górny <mgorny@quansight.com>
rgommers
left a comment
rgommers
left a comment
There was a problem hiding this comment.
This was reviewed in detail by multiple co-authors before opening this PR. It contains important clarifications, as well as the diagram that multiple reviewers found really helpful.
PEP editors, can this please be merged?
There was a problem hiding this comment.
This looks okay in dark mode:
But the lines and a lot of text are invisible or hard to read in light:
There was a problem hiding this comment.
@mgorny I thought we fixed the light/dark mode behavior?
There was a problem hiding this comment.
Weird. It renders fine for me:
There was a problem hiding this comment.
SVG portability ... sigh. On macOS, it renders fine on Safari, and badly on Firefox and Chrome.
There was a problem hiding this comment.
For https://pypackaging-native.github.io/ I took the easy way out and just committed two .svg files, with the selected one depending on dark mode yes/no. Not sure if that's acceptable here. Can also just accept that it's imperfect in one of the modes and commit the white-background one, if that would be okay (there's already another image that's white-only because screenshot).
There was a problem hiding this comment.
That's Firefox on Linux? It doesn't show for me on Chrome/Safari/Firefox/Opera on macOS, or Chrome on Android.
https://pep-previews--4867.org.readthedocs.build/pep-0817/#installing-a-package-from-an-index
If you want to use PNG, you can put :class: invert-in-dark-mode on it to invert via CSS (see other PEPs).
There was a problem hiding this comment.
Two SVG files sound like a good workaround, but I don't know if we can implement switching between them via Sphinx.
Either way, I'm fine with any of the solutions, please decide for me :-).
There was a problem hiding this comment.
If you want to use PNG, you can put
:class: invert-in-dark-modeon it to invert via CSS (see other PEPs).
This sounds nice. PNG is much easier to work with, so if it's that easy, I'd prefer that - will not be the last time we'll need this.
There was a problem hiding this comment.
Done. I've also added the class to PyTorch selector screenshot.
@hugovk, could you check if the same problem applies to the GROMACS bar graph? If yes, I can also replace it with a PNG.
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
3 participants
PEP 123: Summary of changes)This is a set of updates to PEP 817 that were prepared prior to the decision to split it into smaller PEPs. We do not intend to pursue PEP 817 in this updated form. However, we would like to get the changes committed to the repository so that the history of changes is not lost.
The changes include refocusing of the security details, some clarifications, tool behavior outlines and update for the images to respect dark theme.
We are not marking the PEP as withdrawn, as our current plan is to eventually replace the specification part with an index listing the split PEPs.
📚 Documentation preview 📚: https://pep-previews--4867.org.readthedocs.build/pep-0817/