Skip to content

Doc fix nitpick #15034

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 5 commits into from
Aug 12, 2019
Merged

Doc fix nitpick #15034

merged 5 commits into from
Aug 12, 2019

Conversation

tacaswell
Copy link
Member

@tacaswell tacaswell commented Aug 12, 2019

This replaces #15015 .

Closes #15030

tacaswell and others added 2 commits August 11, 2019 21:10
aka some missing references were fixed between the last build of the
nitpicky-mode PR and its merge.
@tacaswell tacaswell added this to the v3.2.0 milestone Aug 12, 2019
@tacaswell tacaswell requested a review from alexrudy August 12, 2019 01:12
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Aug 12, 2019
@ImportanceOfBeingErnest
Copy link
Member

ImportanceOfBeingErnest commented Aug 12, 2019

I pushed the fix for the offending flake line to your fork @tacaswell (hoping that it helps more than it troubles).

In general, however, I have some problems with this nitpicky stuff.

  1. It seems noone is currently able to understand the reasons for (most of) the missing references. They might even be caused by bugs upstream. It is hence likely that they won't be fixed soon. Also, if there are upstream fixes required, they might not help because currently the sphinx version used by matplotlib is pinned.
  2. From now on, each time an edit of any of the files that contain missing references in docstrings will require to edit the line numbers, which is not only cumbersome, but clearly a burden we should not load on contributors' shoulders.
  3. The race condition that led to this trouble here will likely reoccur in the future unless all missing references have been fixed at least once.

So is anyone actually optimistic this can be solved anytime soon? If not, I would propose to disable nitpicky for now.

@alexrudy
Copy link
Contributor

@ImportanceOfBeingErnest I think you are right, but I would propose something slightly different:

The line number bit was suggested by @anntzer in the original PR. I propose we remove the warnings about unused nitpick ignores by line number – but otherwise leave nitpick turned on.

This will still prevent the addition of "new" incorrect references, but allow any existing missing references to persist. The disadvantage is that it will allow "old" missing reference ignores to persist as well.

@tacaswell
Copy link
Member Author

Thanks @ImportanceOfBeingErnest definitely helps more than it hurts!

I'm going to merge this so that our docs build clean again and move discussion about this to a new issue.

@tacaswell tacaswell merged commit e4dbfb5 into matplotlib:master Aug 12, 2019
@tacaswell tacaswell deleted the doc_fix_nitpick branch August 12, 2019 13:22
Copy link
Contributor

@alexrudy alexrudy left a comment

Choose a reason for hiding this comment

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

This LGTM –

The double backtick issues is pervasive in matplotlib, and causes some issues in newer versions of numpydoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doc build broken
6 participants