Skip to content

Conversation

@vstinner
Copy link
Member

@vstinnervstinner commented Jul 25, 2023

Maintaining nitpick_ignore Python list in conf.py is a burden. Add a YAML configuration file which is easier to maintain.


📚 Documentation preview 📚: https://cpython-previews--107278.org.readthedocs.build/

@vstinner
Copy link
MemberAuthor

I prefer YAML over TOML to write a list of strings with bare minimum formating: no quote, no [..., ..., ...]```, just a bullet point - ``.

@vstinner
Copy link
MemberAuthor

Maintaining nitpick_ignore Python list in conf.py is a burden. Add a YAML configuration file which is easier to maintain. Ignore also standard C types in the "c:identifier" domain. Update Doc/requirements-oldest-sphinx.txt.
@AA-Turner
Copy link
Member

Cross-posting from discord: I’d prefer not to create an additional (CPython-custom) YAML file here if possible, perhaps instead we could have a nitpick_ignore.py that we just import into conf.py? I assume the objection to the variable being within conf.py is that the length makes the file unwieldy.

A

@vstinner
Copy link
MemberAuthor

I assume the objection to the variable being within conf.py is that the length makes the file unwieldy.

I would like to add a configuration file since writing a long list of strings in Python if annoying. The YAML file is less verbose.

Also, Standard C types (types section) is duplicated: used in the c:type domain and in the c:identifier domain.

Compare:

 ('c:func', 'calloc'), ('c:func', 'dlopen'), ('c:func', 'exec'), ('c:func', 'fcntl'), ('c:func', 'fork'), ('c:func', 'free'), ('c:func', 'gmtime'), 

to:

 - calloc - dlopen - exec - fcntl - fork - free - gmtime 

@vstinner
Copy link
MemberAuthor

Hum, shown diferently, I have a local branch to complete these nitpick ignore list. I added many functions, macros, etc. and the list is now quite big:

nitpick_ignore= [ # Standard C macros ('c:macro', 'EDOM'), ('c:macro', 'EINTR'), ('c:macro', 'LLONG_MAX'), ('c:macro', 'LLONG_MIN'), ('c:macro', 'LONG_MAX'), ('c:macro', 'LONG_MIN'), ('c:macro', 'SIGINT'), # Standard C variables ('c:data', 'errno'), # Standard environment variables ('envvar', 'BROWSER'), ('envvar', 'COLUMNS'), ('envvar', 'COMSPEC'), ('envvar', 'DISPLAY'), ('envvar', 'HOME'), ('envvar', 'HOMEDRIVE'), ('envvar', 'HOMEPATH'), ('envvar', 'IDLESTARTUP'), ('envvar', 'LANG'), ('envvar', 'LANGUAGE'), ('envvar', 'LC_ALL'), ('envvar', 'LC_CTYPE'), ('envvar', 'LC_COLLATE'), ('envvar', 'LC_MESSAGES'), ('envvar', 'LC_MONETARY'), ('envvar', 'LC_NUMERIC'), ('envvar', 'LC_TIME'), ('envvar', 'LINES'), ('envvar', 'LOGNAME'), ('envvar', 'PAGER'), ('envvar', 'PATH'), ('envvar', 'PATHEXT'), ('envvar', 'SOURCE_DATE_EPOCH'), ('envvar', 'TEMP'), ('envvar', 'TERM'), ('envvar', 'TMP'), ('envvar', 'TMPDIR'), ('envvar', 'TZ'), ('envvar', 'USER'), ('envvar', 'USERNAME'), ('envvar', 'USERPROFILE'), # Win32 API ('c:func', 'FormatMessage'), ('c:func', 'GetFileInformationByHandle'), ('c:func', 'GetLastError'), ('c:func', 'GetVersionEx'), ('c:func', 'MessageBeep'), ('c:func', 'PlaySound'), ('c:func', 'ShellExecute'), ('c:func', 'TerminateProcess'), ('c:func', 'VirtualAlloc'), ('c:func', 'VirtualFree'), ('c:func', 'WSAIoctl'), # Win32 macros ('c:macro', 'CP_ACP'), # Do not error nit-picky mode builds when _SubParsersAction.add_parser cannot# be resolved, as the method is currently undocumented. For context, see# https://github.com/python/cpython/pull/103289. ('py:meth', '_SubParsersAction.add_parser'), ] # Standard C typesc_std_types="""FILE__int64int64_tintmax_toff_tptrdiff_tsiginfo_tsize_tssize_ttime_tuint64_tuintmax_tuintptr_tva_listwcharwchar_t""".strip().split() fornameinc_std_types: nitpick_ignore.append(('c:type', name)) # "c:identifier" domain is used by types in ".. c:function:" definitionsnitpick_ignore.append(('c:identifier', name)) # Standard C functionsc_std_funcs="""_exit_stricmpabortatofcallocclosectimedlopenexecexitfcntlflockforkfreefstatfsyncgettimeofdaygetsidgmtimeinet_atoninet_ptonioctllocaleconvlocaltimelockflstatmainmallocmemmovememcpymktimemmapmunmapopenperrorposix_spawnposix_spawn_file_actions_addcloseposix_spawn_file_actions_adddup2posix_spawn_file_actions_addopenposix_spawnpprintfputenvqsortreallocselectsetenvsetpgidsetpgrpsetsidsetsockoptsigactionsigaltstacksiginterruptsignalsnprintfsplicesprintfstatstatvfsstrcasecmpstrcmpstrerrorstrlenstrncmpsystemunsetenvvsnprintfvsprintf""".strip().split() fornameinc_std_funcs: nitpick_ignore.append(('c:func', name))

Note: this list is incomplete :-)

@vstinner
Copy link
MemberAuthor

This PR fix warnigs like:

c:identifier reference target not found: va_list 

I get this warning more and more on PR changing the C API. Example in a PR changing the C API doc: https://github.com/python/cpython/pull/107274/files

Even if va_list is already in Doc/conf.py, the c:function markup looks for types in c:identifier and so emit a warning that the type is unknown. This PR adds va_list to c:typeandc:identifier domains.

@hugovk
Copy link
Member

I've a slight (non-blocking) preference to avoid adding more code that needs maintaining, and having Sphinx deal with reading config.

Using standard Sphinx things also makes things more familiar for new contributors. And we're also cautious about adding new dependencies (PyYAML) in case downstream redistributors haven't installed them, for example, see sphinxext.opengraph.

I don't mind the existing nitpick_ignore list too much. Moving it to another Python file is okay too.

But it's more important to me to improve the docs, including fixing/ignoring warnings, so if others also like this approach, it's fine by me too.

@serhiy-storchaka
Copy link
Member

It is a shame that Spjinx requires duplicating c:type names as c:identifier.

It can be fixed by a simple code:

forrole, nameinnitpick_ignore: ifrole=='c:type': nitpick_ignore.append(('c:identifier', name)) delname, role

Would not it enough?

For now there is no significant difference between Python and YAML. Inany case we will add only few lines at once and not going to grow the nitpick_ignore list too much. Too much names here increase a chance of using incorrect role with a name from this list (it already happened with LINES and COLS).

@vstinner
Copy link
MemberAuthor

It is a shame that Spjinx requires duplicating c:type names as c:identifier. It can be fixed by a simple code: (...) Would not it enough?

I wrote PR #107295 to implement exactly that (I made minor changes in your proposed code).

@vstinner
Copy link
MemberAuthor

I abandon this PR in favor of PR #107295 (merged) and PR #107301. Thanks for your feedback.

@vstinnervstinner deleted the nitpick_ignore branch July 26, 2023 17:01
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vstinner@AA-Turner@hugovk@serhiy-storchaka@bedevere-bot