-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add core typings #59
Add core typings #59
Conversation
Co-authored-by: Paweł Srokosz <pawel.srokosz@cert.pl>
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 would check if transforming type(x) is str
to isinstance(x, str)
helps mypy to deduce type assertions. If yes, this should be used to avoid some casts.
karton/core/inspect.py
Outdated
def last_update(self) -> int: | ||
return max(task.last_update for task in self.tasks) | ||
def last_update(self) -> float: | ||
return max(task.last_update for task in self.tasks if task.last_update) |
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 would make the last_update non-optional (e.g. initialize with time.time() on construction). If max gets the empty sequence, raises exception ValueError: max() arg is an empty sequence
which might be not expected by user.
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.
Corrected, although it still could raise that exception if self.tasks
is empty
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.
Looks ok 👍
* Add core typings * Sort imports * Format code with black * Reformat with correct black * Fix circular import * Make flake happy * Make mypy happy * Update karton/core/backend.py Co-authored-by: Paweł Srokosz <pawel.srokosz@cert.pl> * Refactor content in ResourceBase constructor * Reformat * Make task.last_update no-noptional * Split long lines * More exceptions * Reformat * Add system typings * Check downloaded task Co-authored-by: Paweł Srokosz <pawel.srokosz@cert.pl>
Closes #53
Closes #60