Skip to content

README.rst to README.md #24067

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

Merged
merged 3 commits into from
Oct 6, 2022
Merged

README.rst to README.md #24067

merged 3 commits into from
Oct 6, 2022

Conversation

grgkaran03
Copy link
Contributor

PR Summary

Change README.rst to README.md. Addressing the issue #24064

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

@jklymak
Copy link
Member

jklymak commented Oct 1, 2022

Thanks for doing this. However, most of our documentation is in rst. I would expect we want to continue doing that.

@timhoffm
Copy link
Member

timhoffm commented Oct 2, 2022

There are arguments for switching. I’m +0.5 on this. I think this needs a discussion. I don’t have time right now, but want to make sure this is not discarded right away.

@story645 story645 added the status: needs comment/discussion needs consensus on next step label Oct 2, 2022
@timhoffm
Copy link
Member

timhoffm commented Oct 2, 2022

Relevance: I assume this page is mainly viewed through the GitHub interface, in which case, both versions are are rendered and it does not make a difference. Markdown vs. reStructuredText only affects

  • people looking into the code of a cloned repo

    • discoverability: README.md is more commonly used in projects, but in the end users will discover README.* equally easy idependent of the extension.
    • familiarity: Some people may be slightly more confused by the .rst extension as reStructuredText is less well known (in particular if you've never written sphinx-based documentation)
    • readability: Both markups can reasonably well be understood without knowing the format exactly. The code for the badges is more compact and thus less distracting in markdown. OTOH, personally I find the ReST section titles more obvious and thus easier to navigate (but if you have an editor with Markdown highlighting there's not a real difference)
  • people who want to edit the readme:

    • consistency @jklymak mentioned "most of our documentation is in rst. I would expect we want to continue doing that." I consider the README rather independent of all the other docs. Technically it is (which is why switching to MD is possible), logically, it's more an entry/overview site for the repo than docs.
    • familiarity: MD is more common. I assume that everybody familiar with ReST, is can also write MD, but not the other way round.

Overall, it does not matter too much, but given all arguments I'm +0.5 for switching to MD.

@story645
Copy link
Member

story645 commented Oct 2, 2022

I assume this page is mainly viewed through the GitHub interface

Also at PyPI because it's now the long description, but according to their docs both versions should work https://packaging.python.org/en/latest/guides/making-a-pypi-friendly-readme/

I find the .md slightly easier to work w/, especially through the github interface where I usually do the readme editing, so I'm also +.5, which is I think it's a good idea but won't put up a fight if someone is strongly against.

README.md Outdated
used in Python scripts, Python/IPython shells, web application servers,
and various graphical user interface toolkits.

## **Install**
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
## **Install**
## Install

I don't think we want bold formatting on top of the heading. Also applies to all following headings.

README.md Outdated

## **Contribute**

You\'ve discovered a bug or something else you want to change -
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
You\'ve discovered a bug or something else you want to change -
You've discovered a bug or something else you want to change -

Escaping not needed in Markdown.

README.md Outdated
You\'ve discovered a bug or something else you want to change -
excellent!

You\'ve worked out a way to fix it -- even better!
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
You\'ve worked out a way to fix it -- even better!
You've worked out a way to fix it -- even better!

Copy link
Member

@story645 story645 left a comment

Choose a reason for hiding this comment

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

this PR can't go in w/o

matplotlib/setup.py

Lines 274 to 275 in 1b3019d

long_description=Path("README.rst").read_text(encoding="utf-8"),
long_description_content_type="text/x-rst",
being edited to the new file (instructions at https://packaging.python.org/en/latest/guides/making-a-pypi-friendly-readme/#including-your-readme-in-your-package-s-metadata) & testing that works. Otherwise the PyPI listing will be broken since this PR deletes README.rst

@grgkaran03
Copy link
Contributor Author

pre-commit.ci autofix

@jklymak
Copy link
Member

jklymak commented Oct 2, 2022

I'm -1 on this simply because if it isn't broke I don't see why we would go through the thrash of changing it. As you have found, there are places that link this as README.rst and keeping both doesn't seem an option.

For better or worse, our docs are in rst. We don't particularly want new contributors adding to our README, so I don't see a strong case for change.

@tacaswell
Copy link
Member

I'm neutral on this. I agree with basically everything Tim and Jody have said. Changing from rst -> md is basically a wash technically and is mostly an aesthetic preference but has a cost / risk to change (and may have downstream cost if packagers are looking for this file (but I do not know for a fact one way or the other)).

That said, the winds of change seem to be blowing in favor of md over rst and it may not be worth the effort to resist. It looks like most of the work seem to be done so there is no sense in throwing it away. I suspect that if we do not take this PR we will get duplicate issues/PRs yearly indefinitely.

@grgkaran03 You provoked a discussion among the maintainers (we are not a monolith!). Please be assured that we appreciate your work no matter which way we go on this.


I think this does get all of our internal usage:

sharon@16:54  ➤  git grep README.rst
.github/PULL_REQUEST_TEMPLATE.md:- [ ] New features have an entry in `doc/users/next_whats_new/` (follow instructions in README.rst there).
.github/PULL_REQUEST_TEMPLATE.md:- [ ] API changes documented in `doc/api/next_api_changes/` (follow instructions in README.rst there).
doc/devel/MEP/index.rst:.. include:: README.rst
doc/devel/contributing.rst:  :file:`doc/users/next_whats_new/README.rst` for more information).
doc/devel/contributing.rst:   :file:`removals`. See :file:`doc/api/next_api_changes/README.rst` for more
doc/missing-references.json:      "doc/users/next_whats_new/README.rst:6"
doc/missing-references.json:      "doc/users/next_whats_new/README.rst:6"
doc/users/prev_whats_new/github_stats_3.3.0.rst:* :ghpull:`15767`: Update badges in README.rst
doc/users/prev_whats_new/github_stats_3.5.0.rst:* :ghpull:`21358`: Backport PR #21357 on branch v3.5.x (DOC: remove test from README.rst)
doc/users/prev_whats_new/github_stats_3.5.0.rst:* :ghpull:`21357`: DOC: remove test from README.rst
setup.py:    long_description=Path("README.rst").read_text(encoding="utf-8"),

@grgkaran03 grgkaran03 requested review from timhoffm and story645 and removed request for timhoffm and story645 October 2, 2022 21:28
@story645 story645 linked an issue Oct 2, 2022 that may be closed by this pull request
@grgkaran03
Copy link
Contributor Author

@timhoffm, please review the corrections also is there any changes to be done before members can come to a decision?

@grgkaran03 grgkaran03 requested review from story645 and timhoffm and removed request for timhoffm and story645 October 4, 2022 18:27
@grgkaran03 grgkaran03 requested a review from story645 October 4, 2022 18:27
@story645
Copy link
Member

story645 commented Oct 4, 2022

How'd the twine check go?

out of scope for this PR, but should we add it to our tests somehow to catch changes to readme?

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Technically, this is good. Thanks @grgkaran03!

Let's put it on the agenda of the dev call, whether we want to switch to Markdown.

@StefRe
Copy link
Contributor

StefRe commented Oct 6, 2022

for the Python Packaging User Guide:

The most common format is reStructuredText with an “rst” extension, although this is not a requirement; multiple variants of Markdown are supported as well


(github can render 9 markup formats)

@tacaswell tacaswell dismissed story645’s stale review October 6, 2022 20:34

The requested changes has been made.

@tacaswell
Copy link
Member

After discussion on the call we have opted to take this. We may have closed #24064 with no action, however this PR got done so we will take it.

This is not an endorsement of moving any other part of our documentation to md/myst.

@tacaswell tacaswell merged commit eb44018 into matplotlib:main Oct 6, 2022
@tacaswell
Copy link
Member

Thank you for your work on this @grgkaran03 and congratulations on your first merged Matplollib PR 🎉 Hopefully we will hear from you again!

@tacaswell tacaswell added this to the v3.7.0 milestone Oct 6, 2022
@story645 story645 mentioned this pull request Oct 6, 2022
1 task
@grgkaran03 grgkaran03 deleted the kg-feature2 branch October 7, 2022 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs comment/discussion needs consensus on next step
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert readme.rst to readme.md
6 participants