-
-
Notifications
You must be signed in to change notification settings - Fork 201
⌨️⚙️ Update torch type checking #1538
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
base: master
Are you sure you want to change the base?
Conversation
@@ -120,5 +120,6 @@ def __call__(self, steps: Sequence[int]) -> Iterator[int]: | |||
|
|||
#: a resolver for checkpoint keepers | |||
keeper_resolver: ClassResolver[CheckpointKeeper] = ClassResolver.from_subclasses( | |||
CheckpointKeeper, default=CheckpointKeeper | |||
CheckpointKeeper, # type:ignore[type-abstract] |
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.
Is that something that can be fixed upstream?
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 did a little looking into mypy and I don't think they support differentiating between abstract subclasses or concrete subclasses. I agree this is not a pretty workaround to use so many type ignores
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.
Why does it even need to distinguish here? from_subclasses
should accept any type
(abstract or non-abstract), right?
if num_entities is None: | ||
num_entities = mapped_triples[:, [0, 2]].max().item() + 1 |
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.
Could be replaced by pykeen.triples.utils.get_num_ids
As a follow-up to #1537, this adds type annotations for torch. This is the most difficult step so far