Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

DOC: Fix missing-reference generation on Windows#26850

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ksunden merged 1 commit intomatplotlib:mainfromQuLogic:fix-missing-refs
Oct 4, 2023

Conversation

@QuLogic
Copy link
Member

PR summary

On Windows, the path is absolute and contains a colon after the drive letter, so splitting on colon results in trying to relativize 'c' (or whatever the drive is) to the base directory, which just makes the final path into base directory + file path.

These missing references all broke in#26628, though I'm not sure why builds never failed since they aren't on Windows, or using the same base paths.

PR checklist

@QuLogicQuLogic added the Documentation: buildbuilding the docs labelSep 21, 2023
@QuLogicQuLogic added this to thev3.8.1 milestoneSep 21, 2023
@QuLogicQuLogic changed the titleDOC: Fix misssing-reference generation on WindowsDOC: Fix missing-reference generation on WindowsSep 21, 2023
@QuLogic
Copy link
MemberAuthor

though I'm not sure why builds never failed since they aren't on Windows, or using the same base paths.

Ah, I think because on load and comparison it runs through:

def_truncate_location(location):
"""
Cuts off anything after the first colon in location strings.
This allows for easy comparison even when line numbers change
(as they do regularly).
"""
returnlocation.split(":",1)[0]

which with the Windows-style paths gives us alwaysdoc/C so everything matches, perhaps?

@story645
Copy link
Member

The missing references file confuses me/I also I think more or less had the linter ignore it - I'm just not sure which of those I'm supposed to care about.

@QuLogicQuLogicforce-pushed thefix-missing-refs branch 3 times, most recently frome1651de tof762b4cCompareSeptember 21, 2023 23:40
@QuLogic
Copy link
MemberAuthor

So it looks like CI is getting additional unused references:

doc/docstring of builtins.list:17: WARNING: Reference py:class HashableList[_HT] for doc/docstring of builtins.list:17 can be removed from missing-references.json. It is no longer a missing reference in the docs.lib/mpl_toolkits/axisartist/axisline_style.py:docstring of mpl_toolkits.axisartist.axisline_style.AxislineStyle:1: WARNING: Reference py:class mpl_toolkits.axisartist.axisline_style._FancyAxislineStyle.FilledArrow for lib/mpl_toolkits/axisartist/axisline_style.py:docstring of mpl_toolkits.axisartist.axisline_style.AxislineStyle:1 can be removed from missing-references.json. It is no longer a missing reference in the docs.lib/mpl_toolkits/axisartist/axisline_style.py:docstring of mpl_toolkits.axisartist.axisline_style.AxislineStyle:1: WARNING: Reference py:class mpl_toolkits.axisartist.axisline_style._FancyAxislineStyle.SimpleArrow for lib/mpl_toolkits/axisartist/axisline_style.py:docstring of mpl_toolkits.axisartist.axisline_style.AxislineStyle:1 can be removed from missing-references.json. It is no longer a missing reference in the docs.lib/matplotlib/path.py:docstring of matplotlib.path:1: WARNING: Reference py:class numpy.uint8 for lib/matplotlib/path.py:docstring of matplotlib.path:1 can be removed from missing-references.json. It is no longer a missing reference in the docs.doc/docstring of builtins.list:17: WARNING: Reference py:obj matplotlib.typing._HT for doc/docstring of builtins.list:17 can be removed from missing-references.json. It is no longer a missing reference in the docs.

but I don't see them locally. I guess that's probably because of some differing versions of things. I'll leave that option off for now.

@story645
Copy link
Member

A possibly good first issue is running through those and commenting out the corresponding ignored_warnings on the linters

@QuLogic
Copy link
MemberAuthor

The missing references file confuses me/I also I think more or less had the linter ignore it - I'm just not sure which of those I'm supposed to care about.

This file causes theWARNING: xyz link target could not be found type of things to be skipped. Mostly they're there for stuff that isn't documented, but linked, e.g., a method with an inherited docstring from a superclass with links to other methods that are not overridden (so the linked methods don't show up in the subclass).

story645 reacted with thumbs up emoji

On Windows, the path is absolute and contains a colon after the driveletter, so splitting on colon results in trying to relativize 'c' (orwhatever the drive is) to the base directory, which just makes the finalpath into base directory + file path.
Copy link
Member

@ksundenksunden left a comment

Choose a reason for hiding this comment

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

The changes look fine, but I am slightly confused as to why weweren't getting warnings for all of the things in that file that had the windows paths.

Are we ignoring the path portion entirely somehow? if so should it just be removed in total from the json file?

I get that we have config on whether or not to warn about unused entries in this file (which is currently set to not warn for those), but presumably many of these are valid entries thatshould be included in the ignores.

@story645
Copy link
Member

we weren't getting warnings for all of the things in that file that had the windows paths

Where were you expecting those to show up?

@ksunden
Copy link
Member

I guess I was expecting the warning that should have been silenced to still show up (and flag as a warning/get extracted by the circle build step etc.

@QuLogic
Copy link
MemberAuthor

Are we ignoring the path portion entirely somehow? if so should it just be removed in total from the json file?

I looked again; it appears that the answer is yes. Only the keys are used for ignoring warnings:
https://github.com/matplotlib/matplotlib/blob/2cbdff865f170fffb868cc12f1265715c0e4f40c/doc/sphinxext/missing_references.py#L285C62-L285C62

The file paths appear to be only to help people to investigate ignored things, and to make the warnings forunused ignores.

@story645story645 reopened thisOct 4, 2023
@ksundenksunden merged commit58646c2 intomatplotlib:mainOct 4, 2023
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestOct 4, 2023
@QuLogicQuLogic deleted the fix-missing-refs branchOctober 4, 2023 19:16
QuLogic added a commit that referenced this pull requestOct 4, 2023
…850-on-v3.8.xBackport PR#26850 on branch v3.8.x (DOC: Fix missing-reference generation on Windows)
@ksundenksunden mentioned this pull requestNov 2, 2023
5 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@story645story645story645 approved these changes

@ksundenksundenksunden approved these changes

Assignees

No one assigned

Labels

Documentation: buildbuilding the docs

Projects

None yet

Milestone

v3.8.1

Development

Successfully merging this pull request may close these issues.

3 participants

@QuLogic@story645@ksunden
[8]ページ先頭
©2009-2025 Movatter.jp