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 dynamically-set __all__ variable#1659

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
Byron merged 8 commits intogitpython-developers:mainfromDeflateAwning:main
Oct 11, 2023

Conversation

DeflateAwning
Copy link
Contributor

@DeflateAwningDeflateAwning commentedSep 12, 2023
edited
Loading

Fixes issue#1656 by statically setting the__all__ variable.
Note that this could probably be done more preciely. The list was generated using the dynamic generation code currently there, printed, and then inserted back in.

Copy link
Member

@ByronByron left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

It looks like the list is not complete though, is this intentional?

Out[19]:['Actor','AmbiguousObjectName','BadName','BadObject','BadObjectType','BaseIndexEntry','Blob','BlobFilter','BlockingLockFile','CacheError','CheckoutError','CommandError','Commit','Diff','DiffIndex','Diffable','FetchInfo','Git','GitCmdObjectDB','GitCommandError','GitCommandNotFound','GitConfigParser','GitDB','GitError','HEAD','Head','HookExecutionError','IndexEntry','IndexFile','IndexObject','InvalidDBRoot','InvalidGitRepositoryError','List','LockFile','NULL_TREE','NoSuchPathError','ODBError','Object','Optional','ParseError','PathLike','PushInfo','RefLog','RefLogEntry','Reference','Remote','RemoteProgress','RemoteReference','Repo','RepositoryDirtyError','RootModule','RootUpdateProgress','Sequence','StageType','Stats','Submodule','SymbolicReference','TYPE_CHECKING','Tag','TagObject','TagReference','Tree','TreeModifier','Tuple','Union','UnmergedEntriesError','UnsafeOptionError','UnsafeProtocolError','UnsupportedOperation','UpdateProgress','WorkTreeRepositoryUnsupported','remove_password_if_present','rmtree','safe_decode','to_hex_sha']In [20]:len(git.__all__)Out[20]:75In [21]:len(all)Out[21]:67

Could you also sort the list please? This makes it easier to maintain I think. Thank you.

@DeflateAwning
Copy link
ContributorAuthor

I simply ran the existing code that was there, and then hard-coded it, removing entries that Pyright said were invalid.

Is this new list the complete list that should be used?

@Byron
Copy link
Member

[..] removing entries that Pyright said were invalid.

I think this explains the mismatch - I created this list based on what's actually there, without looking at a linter. My take on this is that it should be a complete substitution, and no entry can be missing, in order to remain compatible.

Is this new list the complete list that should be used?

Yes, I think so. It would be interesting to know what Pyright is complaining about, but would hope that can be fixed without altering the contents of__all__.

Thanks for bearing with me on this one.

- explicit imports in exc added to avoid linting errors in __init__
@DeflateAwning
Copy link
ContributorAuthor

Implemented these changed, please merge :)

@DeflateAwning
Copy link
ContributorAuthor

DeflateAwning commentedOct 6, 2023
edited
Loading

To fully fix the type hinting issues in all files I've modified here, mergethis PR on gitdb, and bump the submodule version

@Byron
Copy link
Member

It seems the NOQA part doesn't kick in for some reason - maybe this can also be validated locally by use of avenv ortox.

@DeflateAwning
Copy link
ContributorAuthor

Should finally be good now! You'll probably want to squash and merge though, as I had a time stuggling with the linter (some new things were learned though)

@Byron
Copy link
Member

Thanks a lot, it seems correct now.

For future reference, please prefer to rebase instead of merging. Unfortunately my tooling isn't quite up to the task to visualize complex commit graphs nicely (or I don't know how to use them).

@ByronByron merged commit87cc325 intogitpython-developers:mainOct 11, 2023
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestOct 11, 2023
…opers#1659Whengitpython-developers#1659 was updated to pick up linting configuration changes, itinadvertently undid one of the URL changes made ingitpython-developers#1662, puttingthe URL in the git.exe module back to the one that redirects to adifferent BSD license from the one this project uses.Since only that one module was affected, the fix is simple. Thisonly changes the URL back; it doesn't undo any othergitpython-developers#1659 changes.
@DeflateAwning
Copy link
ContributorAuthor

I'll have to figure that out in GitHub Desktop

Byron reacted with thumbs up emoji

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestOct 11, 2023
…opers#1659Whengitpython-developers#1659 was updated to pick up linting configuration changes, itinadvertently undid one of the URL changes made ingitpython-developers#1662, puttingthe URL in the git.exc module back to the one that redirects to adifferent BSD license from the one this project uses.Since only that one module was affected, the fix is simple. Thisonly changes the URL back; it doesn't undo any othergitpython-developers#1659 changes.
Byron added a commit that referenced this pull requestOct 12, 2023
@DeflateAwning
Copy link
ContributorAuthor

Can you do a release with these changes soon?

@Byron
Copy link
Member

@DeflateAwning
Copy link
ContributorAuthor

Thank you!

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestOct 20, 2023
The git.exc module imports exceptions from gitdb.exc to republishthem, as well as defining its own (also for use from outside). Butbecause it did not define __all__, the intent for the exceptions itimported was unclear, since names that are introduced by importsand not present in __all__ are not generally considered public,even when __all__ is absent and a "*" import would reimport them.This rectifies that by adding __all__ and listing both imported andnewly introduced exceptions explicitly in it. Although thisstrictly expands which names are public under typical conventions,it strictly contracts which names are imported by a "*" import,because the presence of __all__ suppresses names not listed in itfrom being imported that way. However, because under typicalconventions those other names are not considered public, and theywere not even weakly documented as public, this should be okay.(Even though this is not a breaking change, in that code it wouldbreak would already technically be broken... if it turns out thatit is common to wrongly rely on the availabiliy of those names,then this may need to be revisited and slightly modified.)This brings the readily identified public interface of git.exc inline with what is weakly implied (and intended) by its docstring.This also modifies __init__.py accordingly: The top-level gitmodule has for some time used a "*" import on git.exc, causingthe extra names originally meant as implementation details to beincluded. Because its own __all__ was dynamically generated untilc862845,gitpython-developers#1659 also added8edc53b to retain the formerly presentnames in __all__. So the change here imports those names from themodules that deliberately provide them, to preserve compatibility.
renovatebot referenced this pull request in allenporter/flux-localOct 20, 2023
[![MendRenovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)This PR contains the following updates:| Package | Change | Age | Adoption | Passing | Confidence ||---|---|---|---|---|---|| [GitPython](https://togithub.com/gitpython-developers/GitPython) |`==3.1.37` -> `==3.1.40` |[![age](https://developer.mend.io/api/mc/badges/age/pypi/GitPython/3.1.40?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/GitPython/3.1.40?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/GitPython/3.1.37/3.1.40?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/GitPython/3.1.37/3.1.40?slim=true)](https://docs.renovatebot.com/merge-confidence/)|---### Release Notes<details><summary>gitpython-developers/GitPython (GitPython)</summary>###[`v3.1.40`](https://togithub.com/gitpython-developers/GitPython/compare/3.1.38...3.1.40)[CompareSource](https://togithub.com/gitpython-developers/GitPython/compare/3.1.38...3.1.40)###[`v3.1.38`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.38)[CompareSource](https://togithub.com/gitpython-developers/GitPython/compare/3.1.37...3.1.38)#### What's Changed- Add missing assert keywords by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1678](https://togithub.com/gitpython-developers/GitPython/pull/1678)- Make clear every test's status in every CI run by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1679](https://togithub.com/gitpython-developers/GitPython/pull/1679)- Fix new link to license in readme by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1680](https://togithub.com/gitpython-developers/GitPython/pull/1680)- Drop unneeded flake8 suppressions by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1681](https://togithub.com/gitpython-developers/GitPython/pull/1681)- Update instructions and test helpers for git-daemon by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1684](https://togithub.com/gitpython-developers/GitPython/pull/1684)- Fix Git.execute shell use and reporting bugs by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1687](https://togithub.com/gitpython-developers/GitPython/pull/1687)- No longer allow CI to select a prerelease for 3.12 by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1689](https://togithub.com/gitpython-developers/GitPython/pull/1689)- Clarify Git.execute and Popen arguments by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1688](https://togithub.com/gitpython-developers/GitPython/pull/1688)- Ask git where its daemon is and use that by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1697](https://togithub.com/gitpython-developers/GitPython/pull/1697)- Fix bugs affecting exception wrapping in rmtree callback by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1700](https://togithub.com/gitpython-developers/GitPython/pull/1700)- Fix dynamically-set **all** variable by[@&#8203;DeflateAwning](https://togithub.com/DeflateAwning) in[https://github.com/gitpython-developers/GitPython/pull/1659](https://togithub.com/gitpython-developers/GitPython/pull/1659)- Fix small[#&#8203;1662](https://togithub.com/gitpython-developers/GitPython/issues/1662)regression due to[#&#8203;1659](https://togithub.com/gitpython-developers/GitPython/issues/1659)by [@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1701](https://togithub.com/gitpython-developers/GitPython/pull/1701)- Drop obsolete info on yanking from security policy by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1703](https://togithub.com/gitpython-developers/GitPython/pull/1703)- Have Dependabot offer submodule updates by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1702](https://togithub.com/gitpython-developers/GitPython/pull/1702)- Bump git/ext/gitdb from `49c3178` to `8ec2390` by[@&#8203;dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1704](https://togithub.com/gitpython-developers/GitPython/pull/1704)- Bump git/ext/gitdb from `8ec2390` to `6a22706` by[@&#8203;dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1705](https://togithub.com/gitpython-developers/GitPython/pull/1705)- Update readme for milestone-less releasing by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1707](https://togithub.com/gitpython-developers/GitPython/pull/1707)- Run Cygwin CI workflow commands in login shells by[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1709](https://togithub.com/gitpython-developers/GitPython/pull/1709)#### New Contributors- [@&#8203;DeflateAwning](https://togithub.com/DeflateAwning) made theirfirst contribution in[https://github.com/gitpython-developers/GitPython/pull/1659](https://togithub.com/gitpython-developers/GitPython/pull/1659)**Full Changelog**:gitpython-developers/GitPython@3.1.37...3.1.38</details>---### Configuration📅 **Schedule**: Branch creation - At any time (no schedule defined),Automerge - At any time (no schedule defined).🚦 **Automerge**: Enabled.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick therebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about this updateagain.---- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, checkthis box---This PR has been generated by [MendRenovate](https://www.mend.io/free-developer-tools/renovate/). Viewrepository job log[here](https://developer.mend.io/github/allenporter/flux-local).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xOS4yIiwidXBkYXRlZEluVmVyIjoiMzcuMTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestFeb 24, 2024
This makes the git.refresh function unambiguously public.git.refresh was already public in the sense that it was explicitlydocumented as appropriate to call from code outside GitPython.However, it had not been included in git.__all__.Because __all__ existed but omitted "refresh", git.refresh hadappeared non-public to automated tools.This also does some cleanup:- It removes a comment that showed how git.__all__ had been defined  dynamically beforegitpython-developers#1659, since with the addition of "refresh",  git.__all__ no longer contains exactly the same elements as that  technique produced (as it examined the module's contents prior to  running the def statement that bound the name "refresh").- With that comment removed, it is no longer necessary to define  __all__ below the imports to show what the dynamic techinque had  operated on. So this moves it up above them in accordance with  PEP-8.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestFeb 24, 2024
This adds comments to entries in git.__all__ for each of theentries that come from the standard library typing module, notingthem as deprecated.These imports were included in __all__ inadvertently due to theway __all__ was dynamically constructed, and placed in __all__explicitly when __all__ became static ingitpython-developers#1659. They are there forbackward compatibility, in case some code relies on them beingthere. But a module is unlikely to rely intentionally on the gitmodule providing them, since they are not conceptually related toGitPython.`from git import *` should not typically be used, since wildcardimports are not generally recommended, as discussed in PEP-8. Butif someone does choose to use it, they would probably benefit lessfrom DeprecationWarning being issued for each of those names thanthey would usually benefit from DeprecationWarning. This could leadto developers deciding not to enable DeprecationWarning when it mayotherwise be useful. For this reason, no attempt is currently madeto issue DeprecationWarning when those names are accessed asattributes of the git module.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 5, 2024
This makes the git.refresh function unambiguously public.git.refresh was already public in the sense that it was explicitlydocumented as appropriate to call from code outside GitPython.However, it had not been included in git.__all__.Because __all__ existed but omitted "refresh", git.refresh hadappeared non-public to automated tools.This also does some cleanup:- It removes a comment that showed how git.__all__ had been defined  dynamically beforegitpython-developers#1659, since with the addition of "refresh",  git.__all__ no longer contains exactly the same elements as that  technique produced (as it examined the module's contents prior to  running the def statement that bound the name "refresh").- With that comment removed, it is no longer necessary to define  __all__ below the imports to show what the dynamic techinque had  operated on. So this moves it up above them in accordance with  PEP-8.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 5, 2024
This adds comments to entries in git.__all__ for each of theentries that come from the standard library typing module, notingthem as deprecated.These imports were included in __all__ inadvertently due to theway __all__ was dynamically constructed, and placed in __all__explicitly when __all__ became static ingitpython-developers#1659. They are there forbackward compatibility, in case some code relies on them beingthere. But a module is unlikely to rely intentionally on the gitmodule providing them, since they are not conceptually related toGitPython.`from git import *` should not typically be used, since wildcardimports are not generally recommended, as discussed in PEP-8. Butif someone does choose to use it, they would probably benefit lessfrom DeprecationWarning being issued for each of those names thanthey would usually benefit from DeprecationWarning. This could leadto developers deciding not to enable DeprecationWarning when it mayotherwise be useful. For this reason, no attempt is currently madeto issue DeprecationWarning when those names are accessed asattributes of the git module.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 9, 2024
This makes the git.refresh function unambiguously public.git.refresh was already public in the sense that it was explicitlydocumented as appropriate to call from code outside GitPython.However, it had not been included in git.__all__.Because __all__ existed but omitted "refresh", git.refresh hadappeared non-public to automated tools.This also does some cleanup:- It removes a comment that showed how git.__all__ had been defined  dynamically beforegitpython-developers#1659, since with the addition of "refresh",  git.__all__ no longer contains exactly the same elements as that  technique produced (as it examined the module's contents prior to  running the def statement that bound the name "refresh").- With that comment removed, it is no longer necessary to define  __all__ below the imports to show what the dynamic techinque had  operated on. So this moves it up above them in accordance with  PEP-8.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 9, 2024
This adds comments to entries in git.__all__ for each of theentries that come from the standard library typing module, notingthem as deprecated.These imports were included in __all__ inadvertently due to theway __all__ was dynamically constructed, and placed in __all__explicitly when __all__ became static ingitpython-developers#1659. They are there forbackward compatibility, in case some code relies on them beingthere. But a module is unlikely to rely intentionally on the gitmodule providing them, since they are not conceptually related toGitPython.`from git import *` should not typically be used, since wildcardimports are not generally recommended, as discussed in PEP-8. Butif someone does choose to use it, they would probably benefit lessfrom DeprecationWarning being issued for each of those names thanthey would usually benefit from DeprecationWarning. This could leadto developers deciding not to enable DeprecationWarning when it mayotherwise be useful. For this reason, no attempt is currently madeto issue DeprecationWarning when those names are accessed asattributes of the git module.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 18, 2024
The submodules being made explicit here are of course Pythonsubmodules, not git submodules.The git.objects.submodule Python submodule (which is *about* gitsubmodules) is made explicit here (as are the names imported fromthat Python submodule's own Python submodules) along with the otherPython submodules of git.objects.Unlike some other submodules, but like the top-level git moduleuntilgitpython-developers#1659, git.objects already defined __all__ but it wasdynamically constructed. As with git.__all__ previously (as notedingitpython-developers#1859), I have usedhttps://github.com/EliahKagan/deltall hereto check that it is safe, sufficient, and probably necessary toreplace this dynamically constructed __all__ with a literal list ofstrings in which all of the original elements are included. See:https://gist.github.com/EliahKagan/e627603717ca5f9cafaf8de9d9f27ad7Running the modattrs.py script, and diffing against the output frombefore the current import refactoring began, reveals two changes tomodule contents as a result of the change here:- git.objects no longer contains `inspect`, which it imported just  to build the dynamically constructed __all__. Because this was  not itself included in that __all__ or otherwise made public or  used out of git.objects, that is okay. This is exactly analogous  to the situation in8197e90, which removed it from the top-level  git module aftergitpython-developers#1659.- The top-level git module now has has new attributes blob, commit,  submodule, and tree, aliasing the corresponding modules. This  has happened as a result of them being put in git.objects.__all__  along with various names imported from them. (As noted in some  prior commits, there are tradeoffs associated with doing this,  and it may be that such elements of __all__ should be removed,  here and elsewhere.)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ByronByronByron requested changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@DeflateAwning@Byron

[8]ページ先頭

©2009-2025 Movatter.jp