Comments (3)
Thanks for highlighting these possible concerns.
Since naming is hard, and those names didn't strike me as problematic, I'd think it's good to go with them as the new state of affairs is certainly better than the previous one.
from gitpython.
Thanks a lot for documenting these issues with the diff
API. Creating an API like this was clearly a mistake as it's not helping writing more readable code at all due to the overloadedness of the input arguments, all that while 'leveraging' the class hierarchy.
Thus I am not surprised that this is notoriously difficult to annotate, but am happy that this seems to be possible.
My argument for making this change is to achieve greater correctness without breaking anything at runtime, and not that I am at all confident nobody will have to change their code to keep it passing static checks.
I'd argue that a failing mypi check won't constitute as breaking change, it's a grey-zone I am OK to walk into if it one day allows type-checkers to make using GitPython easy (due to them actually being accurate). This is one of the few areas where GitPython can still change as well. Knowing that such a change might break somebodies CI, the only constraint I'd apply to myself is that such 'breaking' changes shouldn't be done lightheartedly.
from gitpython.
Knowing that such a change might break somebodies CI, the only constraint I'd apply to myself is that such 'breaking' changes shouldn't be done lightheartedly.
Speaking of this, in #1859 I should probably have raised the question of what to call the type of the enumeration in git.diff
in which NULL_TREE
and INDEX
(the latter being what Diffable.Index
now aliases) are defined. I called it DiffConstants
. I mentioned one aspect of this in 65863a2:
the name DiffConstants (rather than, e.g., Constants) should avoid confusion even if it ends up in another scope unexpectedly.
There is also the question of singular and plural. This enumeration is named as a namespace of the constants it defines, rather than in the style one would ordinarily name a type: it is awkward to say "NULL_TREE
is a DiffConstants
". Since enumerations always define constants, typically they shouldn't have Constants
or Constant
in their type names, but here it seemed to help distinguish it from everything else in git.diff
. In some languages there are strong conventions for naming enumerations that help resolve this sort of thing; for example, in C#, enumerations should have plural names when they provide flags (and have the [Flags]
attribute) and should have singular names otherwise. I don't know of such conventions for Python.
Possibilities like DiffSpecial
did not seem better. Anything without Diff
in its name would be confusing when accessed directly in git
. The top-level git
module currently uses wildcard imports for almost everything in GitPython, but that can and should change even separately from this, so one approach could be to change that, at least for its imports from git.diff
, so as not to list that enumeration's type: NULL_TREE
and INDEX
should still be imported in git.__init__
and listed in git.__all__
, because NULL_TREE
was there before, but DiffConstants
could defensibly not be imported in git.__init__
.
(In contrast, code using GitPython would have trouble annotating its own functions--if it needs to use this type--if the type were nonpublic, which its exclusion from the subpackage git.diff.__all__
would signify unless otherwise documented, and would appear to signify even if otherwise documented.)
If this is to be changed, it would be best changed before the next release, after which the old name may have to be kept as an alias of the new name in order to avoid breakage. (Removing the old name after a release would probably be an actual breaking change because it would not just break type checking but would also cause runtime errors to occur in imports and elsewhere, in reasonable code written after the first name was added and before it was removed.)
To be clear, I'm not saying this needs to change, only that there may be a benefit to considering the name and deciding if it does.
from gitpython.
Related Issues (20)
- Note in GitCmdObjectDB should be removed or updated HOT 1
- Author name returned as 'empty' HOT 1
- The API Reference documents too few modules HOT 2
- assert_never raises different exception than documented HOT 1
- Commit_ish is much broader than commit-ish HOT 4
- switch to Ruff instead of `flake8` HOT 6
- origin.pull(rebase=True) sometimes lost local commit , gitpyton version 3.1.31 HOT 3
- Wrong type annotation on `Index.commit` for `parent_commits` HOT 3
- Using create submodule doesn't create the correct submodule structure HOT 4
- Submodule.__init__ `parent_commit` conversion/validation is implied but not done HOT 8
- Module docstring says TagReference but means TagObject
- Top-level refresh function not listed in __all__ HOT 1
- Bad git executable HOT 2
- Unable to specify lines of context while doing diff (-U flag) HOT 1
- γremote().exist()γ exception when lack remote section in config file
- Unhandled IndexError when calling .read() on a malformed config file HOT 6
- Detailed xfail output sometimes distracts from unexpected failures
- Some xfail markings fail to validate their exception types HOT 1
- How can i set create_patch=true, and meanwhile get change_type in Diff
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
π Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. πππ
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google β€οΈ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gitpython.