Skip to content

PR: Transform sorting functions from md to rst file#333

Merged
kgryte merged 10 commits into
data-apis:mainfrom
steff456:sorting-rst
Dec 6, 2021
Merged

PR: Transform sorting functions from md to rst file#333
kgryte merged 10 commits into
data-apis:mainfrom
steff456:sorting-rst

Conversation

@steff456

@steff456 steff456 commented Nov 10, 2021

Copy link
Copy Markdown
Member

This PR rewrites the sorting functions markdown file in a rst file.

Ref: #283

@steff456 steff456 requested review from kgryte and rgommers November 10, 2021 22:00
@steff456 steff456 self-assigned this Nov 10, 2021
@steff456 steff456 added the Maintenance Bug fix, typo fix, or general maintenance. label Nov 10, 2021
Comment thread spec/API_specification/signatures/_sorting_functions.py Outdated
Comment thread spec/API_specification/signatures/_sorting_functions.py Outdated
@rgommers

Copy link
Copy Markdown
Member

There's still an issue with the rendered docs:

image

signatures._sorting_functions. needs to go here.

@rgommers rgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like a nice start, thanks @steff456!

Just a note here that we had a discussion about type annotations and whether they should be reflected in the function signatures or not (this is controlled by autodoc_typehints, see https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html). We thought it's good to start with the doing both - our type annotations are very clean, so it hopefully will look good in signatures.

@asmeurer

Copy link
Copy Markdown
Member

signatures._sorting_functions. needs to go here.

Do you mean the extra space after the ., or something else? The space is probably related to #139, which we will either have to fix by switching to another Sphinx theme or figuring out how to fix it upstream.

@kgryte

kgryte commented Nov 15, 2021

Copy link
Copy Markdown
Contributor

@asmeurer I think he meant the entirety of signatures._sorting_functions. so that the spec just shows sort(...).

@steff456

Copy link
Copy Markdown
Member Author

This PR is ready for review again!

Now the page of each function is shown like this,
image

@steff456

steff456 commented Dec 1, 2021

Copy link
Copy Markdown
Member Author

With the latest changes, now the type hint for the return type is shown as the parameters and the Return type section is removed.

image

I think now this is ready!

@kgryte

kgryte commented Dec 6, 2021

Copy link
Copy Markdown
Contributor

Based on the screenshot and looking through the changes, looks good to me.

@kgryte kgryte added this to the v2022 milestone Dec 6, 2021
@kgryte

kgryte commented Dec 6, 2021

Copy link
Copy Markdown
Contributor

I think we can go ahead and merge this, and, if there are any further changes, we can address in follow-up PRs. Thanks, @steff456!

@kgryte kgryte merged commit 657a8f4 into data-apis:main Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Maintenance Bug fix, typo fix, or general maintenance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants