-
-
Notifications
You must be signed in to change notification settings - Fork 780
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
showbase: Add type annotations to ShowBase/ClientRepositoryBase attributes #1217
base: master
Are you sure you want to change the base?
showbase: Add type annotations to ShowBase/ClientRepositoryBase attributes #1217
Conversation
cc579fe
to
8f972cf
Compare
8f972cf
to
72166c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Needs rebase
- See inline comments
- Do the annotations all work with Python 3.6?
self.mouse2cam = None | ||
self.buttonThrowers = None | ||
self.mouseWatcher = None | ||
self.pipeList: List[Optional[GraphicsPipe]] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be Optional? Will there ever be a None added to the list?
self.cTravStack = Stack() | ||
# Ditto for an AppTraverser. | ||
self.appTrav = 0 | ||
self.appTrav: Optional[CollisionTraverser] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think appTrav is meant to be a CollisionTraverser, but I don't know what it's meant to be - I would suggest leaving this Any for now
@@ -275,7 +278,7 @@ def __init__(self, fStartDirect=True, windowType=None): | |||
|
|||
#: The global :class:`~panda3d.core.GraphicsEngine`, as returned by | |||
#: GraphicsEngine.getGlobalPtr() | |||
self.graphicsEngine = GraphicsEngine.getGlobalPtr() | |||
self.graphicsEngine: GraphicsEngine = GraphicsEngine.getGlobalPtr() | |||
self.graphics_engine = self.graphicsEngine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add an annotation to the underscored version as well?
# Typing annotations | ||
from typing import Optional, TYPE_CHECKING | ||
if TYPE_CHECKING: | ||
from direct.distributed.TimeManager import TimeManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about such an optional import - and is it even necessary since you referred to it by string?
self.win = None | ||
self.frameRateMeter = None | ||
self.sceneGraphAnalyzerMeter = None | ||
self.win: Optional[GraphicsWindow] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this may also be a GraphicsBuffer, in offscreen mode. But it should in any case be a GraphicsOutput.
Ping. Still interested in this? |
Issue description
Currently, a lot of attributes contained on ShowBase have properties with non-inferable types, and as such these attributes nor their methods/attributes are hinted in IDEs.
Solution description
This MR adds type annotations to most properties defined during instance initialization for ShowBase (and a few for ClientRepositoryBase), so that developers which access these properties can determine easily determine the type of the attribute and the properties of its methods (if type information exists for said type, i.e.
panda3d.core
types).A slight caveat is that I did not add typehinting for a few attributes declared in ShowBase, (e.g.
ShowBase.texmem
) because their types are imported viaimportlib
, and I did not want to break the functionality that is provided via using that import method.Another caveat is that if the names used in type annotations are removed (e.g. the
from panda3d.core import *
is changed to import a list of specific names), types imported for annotations (i.e. if they are moved into aif TYPE_CHECKING:
block) will need to be modified so that they are a string literal (e.g.Optional["NodePath"]
) unlessannotations
is imported from__future__
(Python 3.7+) or direct is running on Python 3.11+.I've done my best to verify these type annotations are correct, but please correct me if I have gotten anything wrong.
Checklist
I have done my best to ensure that…