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

Fix name collision#2060

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

Open
NMertsch wants to merge2 commits intogitpython-developers:main
base:main
Choose a base branch
Loading
fromNMertsch:2023-fix-gitconfigparser-autodoc

Conversation

NMertsch
Copy link

The bug

Ingit.config.MetaParserBuilder.__new__:

def__new__(name,bases,clsdict):# <- `name` is the class name    ...forname,methodinmethods:# <- `name` is the current method name        ...returnsuper().__new__(cls,name,bases,clsdict)# <- `name` is the name of the last method in clsdict

Downstream effects

  • git.config.GitConfigParser.__name__ is"write", not"GitConfigParser"
  • Sphinx autodoc misinterprets the nature ofgit.config.GitConfigParser as an alias of itself:
  • image

Fixes#2023

@Byron
Copy link
Member

Thanks, this PR would be great to land I am sure!

Do you have an intuition as to why CI is failing here?

@NMertsch
Copy link
Author

NMertsch commentedJul 20, 2025
edited
Loading

Yes. There was an rst syntax error in CPython's configparser module:https://github.com/python/cpython/blob/v3.8.10/Lib/configparser.py#L768:

- `vars'+ `vars`

configparser.RawConfigParser is the baseclass ofGitConfigParser (the class which is now being documented due to the bugfix), so itsget method (the one with the syntax error) is part of the public interface ofGitConfigParser.

This only occurs in the pipelines for 3.8 and 3.9, so I guess it was fixed later but the fix was never backported.

I'm not sure what's the best way to handle this. I can think of multiple options, but I dislike all of them:

  1. remove-W fromSPHINXOPTS indoc/Makefile to not treat warnings as errors in the whole project.
  2. modify the offending file in the Docs CI jobs before callingsphinx-build.
  3. suppressing the warning type withsuppress_warnings = ["..."] (docs). There is no way to suppress warnings only for specific files or lines, so this would disable the syntax check in the whole project. Also, I can't find the warning code in the error output, but I can find that out if you choose this option. This is a docutils warning, and sphinx has no way of suppressing it.

@NMertsch
Copy link
Author

NMertsch commentedJul 20, 2025
edited
Loading

Here is how this would render without-W (treat warnings as errors):
image

It is not beautiful, but people will understand it.

If I understand correctly, this project is mostly in maintenance-only mode, so the risk of such issues being newly introduced to GitPython itself is fairly low. Maybe that's the best of all the bad options?

@NMertsch
Copy link
Author

Sorry for causing this mess, I didn't expect any negative side effects from the bugfix.

If you want to close this PR, just go ahead, I don't have strong feeling about this and just wanted to contribute a quick win.

If you want me to investigate further in a specific direction, please let me know and I'll see what is feasible for me.

Thanks for this library, btw! :)

@Byron
Copy link
Member

Byron commentedJul 20, 2025
edited
Loading

I see - the fix now causes the configuration to be pulled in correctly, and that results in a conflict in the way a docstring is written in the library or code we pull in.

The issue itself, now that I understand it, is also amazing as it's one of these inconsistencies that shouldn't even be possible. How can a variable in a for-loop survive the for-loop, and permanently overwrite a variable in the outer scope?

Let's go with the fix, and let only errors be errors during documentation generation.

Just one more request: could you add to the body of the most recent commit why the change was made? Just copy-paste a link to the discussion here, or better yet, copy-paste the respective comment text.

Thanks again.

Here is a copilot suggestion, but it might lack detail.

Remove -W from SPHINXOPTS due to warnings triggered by quoting of 'vars'The Sphinx build was failing with -W enabled because warnings were generated related to the way the word 'vars' is quoted in docstrings. Treating warnings as errors (-W) caused otherwise harmless docstring quoting issues to block documentation builds. Removing -W allows the build to succeed despite these warnings.
NMertsch reacted with thumbs up emoji

@NMertschNMertschforce-pushed the2023-fix-gitconfigparser-autodoc branch fromcb77830 to5e7d005CompareJuly 20, 2025 12:15
NMertsch added a commit to NMertsch/GitPython that referenced this pull requestJul 20, 2025
Workaround forpython/cpython#100520 (rst syntax error in configparser docstrings), which was fixed in CPython 3.10+.Docutils raises warnings about the invalid docstrings, and `-W` instructs sphinx to treat this as errors. We can't control or silence these warnings, so we accept them and don't treat them as errors.See the discussion ingitpython-developers#2060 for details.
Workaround forpython/cpython#100520 (rst syntax error in configparser docstrings), which was fixed in CPython 3.10+.Docutils raises warnings about the invalid docstrings, and `-W` instructs sphinx to treat this as errors. We can't control or silence these warnings, so we accept them and don't treat them as errors.See the discussion ingitpython-developers#2060 for details.
@NMertschNMertschforce-pushed the2023-fix-gitconfigparser-autodoc branch from5e7d005 to80fd2c1CompareJuly 20, 2025 12:18
@NMertsch
Copy link
Author

Should be good now, please let me know if there is more to do.
Thank you for the quick and kind responses, I really appreciate it.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docs: No API Docs for git.config.GitConfigParser
2 participants
@NMertsch@Byron

[8]ページ先頭

©2009-2025 Movatter.jp