Add support for conditional blocks in README.rst #6901
Conversation
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.87% 97.87%
=======================================
Files 1084 1086 +2
Lines 94495 94554 +59
=======================================
+ Hits 92483 92542 +59
Misses 2012 2012 ☔ 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
|
Do we actually still need this, since we dumped the cryostat image? |
Unfortunately, we do; with GitHub's implementation of reStructuredText format, it's just not possible to center elements on the page (such as the Cirq logo, the TOC, the table of related software) on both GitHub and PyPI without processing the file somehow. The problem comes from the fact that GitHub's rST processor simply ignores FWIW, the added code is pretty short, and the changes to setup.py are pretty minimal. I do realize, though, that it's more code to maintain, and that it's a bit extreme to try so hard for the sake of some formatting. OTOH, I feel that the result is more professional-looking, with a level of polish that Quantum AI deserves. It would be possible to avoid the processing step if we switched to Markdown. I think @pavoljuhas felt switching to Markdown would be problematic, though. |
|
Another question. There's no mandate for rst format. |
|
AFAICT this is so far only needed to support centered formatting in #6903. TBH, I don't feel that centered format is worth the extra code to maintain and an extra complexity in the README.rst text and in setup.py. If contributors do not pair up their markers correctly or manage to enter a different triangle symbol, they'd get a CI failure which may be annoying. Personally, the left-aligned default format looks good to me. The current README.rst is quite humanly readable in its plain text form. |
|
Yeah, I don't know if this is worth it for centered text, but I will leave it up to you. |
|
Thank you for the feedback! GitHub comments lacks a good reply mechanism, so here I'll try to address multiple questions in one place. From @dstrain115:
Switching to md would help and make things much simpler; in fact, I previously proposed using md in the (now-closed) PR #6805. However, in a chat conversation a couple of months ago, @pavoljuhas requested that the top-level README and the module README files be kept in rST format. I regret the time-limited chat history doesn't extend back far enough, so now I can't refer back to the details, but IIRC it was due to some issue with project descriptions on PyPI. I followed Pavol's request and kept rST. PyPI accepts project descriptions in Markdown format, and both GitHub and PyPI respond to adding a subset of raw HTML into Markdown, so all the formatting that this rST stuff achieves could be done directly in Markdown. Doing so would avoid the need for the filtering code and those hacky comments-for-block-delimiters inside the files. @Pavol I'm sorry to have to ask, but could you re-explain the rST vs md problem? From @Pavol:
Yes, that's right. (Although the filtering mechanism is more general and could permit other things, the centering problem was the driver.)
I agree it's a lot of work for centering. (I think Edvard Munch's painting The Scream was actually the result of his frustrations in trying to make make an RST file work on both GitHub and PyPI.)
If we switch to Markdownn, it would still require paired delimiters, but they would be simpler: <div align="center">
... stuff ...
</div>Would this be easy enough for people? (BTW, the comment string delimiter is arbitrary. I only picked those characters to make the strings unique, and assumed that people would simply copy-paste the things.)
I do accept that aesthetic judgements may differ. For myself, I find some other related projects' README files much more attractive (e.g., https://github.com/PennyLaneAI/pennylane) and am, well, saddened when I see Cirq's. It deserves better, so I'm trying hard to improve ours.
Yes, I agree the result is not as readable. If the use of |
There was a problem hiding this comment.
@mhucka - I pushed the README from #6903 to temporary branches in my fork with and without the extra formatting markup and they render the same with the exception of centering of the top title and of the table in the Integrations section.
I also don't have a good recollection of our chat regarding the rst to markdown conversion in #6805;
perhaps my impression was there would be not much change in the README content and I was not convinced that the change of markup language is worth the effort. (Note we'll need to also update all instances of "README.rst" in the docs and any setup scripts). Now that #6903 makes significant changes to the README text, that point is moot.
If you feel strongly about centering and markdown can provide it out of the box we should go for it instead.
I'd strongly prefer to keep the README code simple and readable in its plain form and avoid custom conditionals and text post-processing.
Perhaps we could get there in these steps
- close this PR (or limit it to fixups in setup.py)
- finalize and merge Overhaul top-level README.rst file #6903 without GitHub-only blocks
- make a follow-up PR that would convert README.rst --> README.md (without text change) and update any dependent code and docs to use
README.md
WDYT?
|
I will
|
This is a major overhaul of the main Cirq README file. Much of the content is rewritten, and there are numerous additions and enhancements all over. The changes include but are not limited to: - Updates and corrections - Addition of new sections - Centering and resizing the logo for a more professional look - Addition of GitHub badges for extra "pizzazz" - Addition of pointers to related Quantumlib software - Addition of a table of contents - Improvements to info about how to cite Cirq - Conversion to Markdown format to allow formatting that is compatible with both GitHub and PyPI This is a follow-on to now-closed PRs quantumlib#6903 and quantumlib#6901, which concerned making roughly the same changes while keeping with the reStructuredText format of the previous README file. Switching to Markdown allows use of some simple raw HTML to achieving formatting that is not possible in pure Markdown, and still do so in a way that works on both GitHub and PyPI.
This is a major overhaul of the main Cirq README file. Much of the content is rewritten, and there are numerous additions and enhancements all over. The changes include but are not limited to: - Updates and corrections - Addition of new sections - Centering and resizing the logo for a more professional look - Addition of GitHub badges for extra "pizzazz" - Addition of pointers to related Quantumlib software - Addition of a table of contents - Improvements to info about how to cite Cirq - Conversion to Markdown format to allow formatting that is compatible with both GitHub and PyPI This is a follow-on to now-closed PRs quantumlib#6903 and quantumlib#6901, which concerned making roughly the same changes while keeping with the reStructuredText format of the previous README file. Switching to Markdown allows use of some simple raw HTML to achieving formatting that is not possible in pure Markdown, and still do so in a way that works on both GitHub and PyPI.
* Overhaul top-level README and convert to Markdown This is a major overhaul of the main Cirq README file. Much of the content is rewritten, and there are numerous additions and enhancements all over. The changes include but are not limited to: - Updates and corrections - Addition of new sections - Centering and resizing the logo for a more professional look - Addition of GitHub badges for extra "pizzazz" - Addition of pointers to related Quantumlib software - Addition of a table of contents - Improvements to info about how to cite Cirq - Conversion to Markdown format to allow formatting that is compatible with both GitHub and PyPI This is a follow-on to now-closed PRs #6903 and #6901, which concerned making roughly the same changes while keeping with the reStructuredText format of the previous README file. Switching to Markdown allows use of some simple raw HTML to achieving formatting that is not possible in pure Markdown, and still do so in a way that works on both GitHub and PyPI. * Adapt setup.py to work with new Markdown-format README * Update to reference README.md instead of README.rst * Use md reference links for better plain-text readability No content edits (except possibly one or two trivial changes of wording). This changes some links to use Markdown "reference" link syntax, to make some of the source text more readable. * Fix list indentation to follow Google Markdown style * Fix links to bib records & reposition recontributors number The previous links for the bibliographic records went to Cirq 1.2 in Zenodo. There doesn’t seem to be a way to get Zenodo to produce bibtex for the latest version of a record (only specific versions of a record), so I ended up using doi.org, but that one doesn't have a way to produce MarcXML. So I removed that format, leaving the other two. This also moves the GitHub contributors badge/count, per recommendation by Pavol in the review of this PR. * Remove README.rst * Add link target for #how-to-cite This is so that existing links go to same section despite the renamed heading.
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:: htmlas 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.pyfile.