Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork948
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
base:main
Are you sure you want to change the base?
Fix name collision#2060
Conversation
Thanks, this PR would be great to land I am sure! Do you have an intuition as to why CI is failing here? |
NMertsch commentedJul 20, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 of 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:
|
NMertsch commentedJul 20, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 commentedJul 20, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
|
cb77830
to5e7d005
CompareWorkaround 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.
5e7d005
to80fd2c1
CompareShould be good now, please let me know if there is more to do. |
The bug
In
git.config.MetaParserBuilder.__new__
:Downstream effects
git.config.GitConfigParser.__name__
is"write"
, not"GitConfigParser"
git.config.GitConfigParser
as an alias of itself:Fixes#2023