-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add support for conditional blocks in README.rst #6901
Open
mhucka
wants to merge
4
commits into
quantumlib:main
Choose a base branch
from
mhucka:mhucka-add-rst-filtering
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Due to differences in reStructuredText support between GitHub and PyPI, some basic things like centering images are impossible to do purely in RST in such a way that it works on both GitHub and PyPI.:(This is not the case with Markdown; both PyPI and GitHub support some limited HTML inside Markdown, and that makes it possible to have the same Markdown README file work on both PyPI and GitHub.) Since we are sticking with RST format for the top-level module README files, I wanted to have a way to create an attractive project README file for both GitHub and PyPI without having to keep two separate files. However, that meant solving the following problem. PyPI supports RST's `.. class::` directive, but GitHub does not. PyPI also respects `:align:` attributes for images, but GitHub does not. Consequently, the only way to center text or images in RST for GitHub is to use raw HTML via the RST `.. raw::` directive, but it turns out that PyPI not only _doesn't support `raw`_ – it actually _throws an error and refuses your upload_ if your README file contains `.. raw::` constructs. What to do? Well, we _can_ use `.. raw:: html` as long as we remove those constructs in the version of the file we save in the distribution for PyPI. Further, since GitHub simply ignores or strips out RST directives like `.. class::`, we can leave _those_ constructs in the file that is sent to PYPI, and make use of them to do what we can't do in GitHub purely in RST. So, in summary: 1. Make the unmodified README.rst file work for GitHub, because that's what people will see when they visit the repo on GitHub. 2. Filter the README.rst file when producing distributions for PyPI, to remove content delimited by specific start and end markers that indicate parts that are valid on GitHub but not PyPI. 3. Use normal RST constructs that work on PyPI but not GitHub to accomplish what we can't do for GitHub without using raw HTML. That's the approach is used in the new updated top-level README.rst file for Cirq, and it is enabled by this small modification to the `setup.py` file.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6901 +/- ##
=======================================
Coverage 97.86% 97.86%
=======================================
Files 1084 1086 +2
Lines 94225 94284 +59
=======================================
+ Hits 92215 92273 +58
- Misses 2010 2011 +1 ☔ View full report in Codecov by Sentry. |
Need to provide `tempfile.NamedTemporaryFile` with the argument `encoding='utf-8'` or else on Windows, we get an exception when working with delimiters that contain uncommon Unicode characters: ``` def encode(self, input, final=False): > return codecs.charmap_encode(input,self.errors,encoding_table)[0] E UnicodeEncodeError: 'charmap' codec can't encode characters in position 19-23: character maps to <undefined> ```
It turns out that on Windows, you can't open a temp file while inside a the context handler "with tempfile.NamedTemporaryFile...". See https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Due to differences in reStructuredText support between GitHub and PyPI, some basic things like centering images are impossible to do purely in RST in such a way that it works on both GitHub and PyPI. (This is not the case with Markdown; both PyPI and GitHub support some limited HTML inside Markdown, and that makes it possible to have the same Markdown README file work on both PyPI and GitHub.)
Since we are sticking with RST format for the top-level module README files, I wanted to have a way to create an attractive project README file for both GitHub and PyPI without having to keep two separate files. However, that meant solving the following problem.
PyPI respects RST's
:align:
attributes for images, but GitHub ignores it. PyPI also supports RST's.. class::
directive, but GitHub ignores it. Consequently, the only way to center text or images in RST for GitHub is to use raw HTML via the RST.. raw::
directive, but here's the catch: it turns out that PyPI not only doesn't supportraw
– it actually throws an error and refuses your upload if your README file contains.. raw::
constructs.What to do? Well, we can use
.. raw:: html
as long as we remove those constructs in the version of the file we save in the distribution for PyPI. Further, since GitHub simply ignores or strips out RST directives like.. class::
, we can leave those constructs in the file that is sent to PYPI, so that at least they do what we want them there. So, in summary:Make the unmodified README.rst file work for GitHub, because that's what people will see when they visit the Cirq repo on GitHub.
Process the README.rst file when producing distributions for PyPI, to remove content delimited by specific start and end markers that indicate parts that are valid on GitHub but not PyPI.
Use normal RST constructs that work on PyPI but not GitHub (that is, leave them in the file), to accomplish on PyPI what we can't do for GitHub without using raw HTML.
That's the approach is used in the new updated top-level README.rst file for Cirq, and it is enabled by this small modification to the
setup.py
file.