Skip to content

Fix bulleted list indentation in docstrings #5250

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 6 commits into from
May 3, 2021

Conversation

andersy005
Copy link
Member

@andersy005 andersy005 commented May 3, 2021

As I understand it, the issue described in #5248 is caused by bad indentation... With this PR, we get the following:

Screen Shot 2021-05-02 at 11 06 41 PM

@max-sixty, let me know if there are other instances that need to be fixed as part of this PR....

@max-sixty
Copy link
Collaborator

max-sixty commented May 3, 2021

That looks great! Thanks!

If there's a way of auto-failing anything that violates this, that would keep it good. But maybe the previous text is indistinguishable from normal prose...

Not worth worrying about at all — really — FYI this looks a tiny bit inconsistent:
image

@andersy005
Copy link
Member Author

@max-sixty, the inconsistencies are now fixed...

@andersy005
Copy link
Member Author

andersy005 commented May 3, 2021

If there's a way of auto-failing anything that violates this, that would keep it good. But maybe the previous text is indistinguishable from normal prose...

Not sure there's a way to automatically detect these issues :(

Copy link
Collaborator

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

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

Perfect!

@keewis
Copy link
Collaborator

keewis commented May 3, 2021

the initial issue, at least, would have been caught by velin

@andersy005
Copy link
Member Author

the initial issue, at least, would have been caught by velin

Right... In a previous comment #4872 (comment) you mentioned that the primary reason the velin hook was deactivated is because of the package maturity. Have you had a chance to test drive it lately? Is there a timeline on when we should activate it?

# - repo: https://github.com/Carreau/velin
# rev: 0.0.8
# hooks:
# - id: velin

@max-sixty
Copy link
Collaborator

Maybe it's time to let Velin spring free! @keewis up to you, as ever

@max-sixty
Copy link
Collaborator

@andersy005 I'll hit the button here, if that's OK with you?

@max-sixty max-sixty merged commit da20d9c into pydata:master May 3, 2021
@andersy005 andersy005 deleted the docs/fix-docstrings branch May 3, 2021 23:33
@andersy005
Copy link
Member Author

Thanks, @max-sixty

@max-sixty
Copy link
Collaborator

Thank you @andersy005 !

@keewis
Copy link
Collaborator

keewis commented May 4, 2021

Have you had a chance to test drive it lately?

no, I didn't.

Maybe it's time to let Velin spring free!

I'll need to investigate more, but I think we should probably do that soon. There should be standard rst linters, too, but those might need more configuration: sphinx adds a lot of new directives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Appearance of bulleted lists in docs
3 participants