-
Notifications
You must be signed in to change notification settings - Fork 1k
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
WIP (wait for sqlalchemy2-stubs): Added types to lib/galaxy/model/__init__.py #12971
Conversation
8602bae
to
a56dbaf
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.
I would advise against typing model/__init__.py
for now (or at least everything that is backed by a database column), I think this will heavily conflict with typing when the sqlalchemy2-stubs are installed (which don't seem to be quite ready yet).
update_time: datetime | ||
create_time: datetime |
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.
That'll conflict with the types defined by the sqlalchemy models
@@ -309,6 +316,7 @@ def _serialize(self, id_encoder: IdEncodingHelper, serialization_options: Serial | |||
|
|||
|
|||
class HasName: | |||
name: str |
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.
That'll conflict with the types defined by the sqlalchemy models
id: int | ||
tool_id: int |
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.
That'll conflict with the types defined by the sqlalchemy models
@mvdbeek Thank you, that makes sense. I'll update this PR after sqlalchemy2-stubs installed |
The original issue is closed common-workflow-lab#137 |
Why: common-workflow-lab#137
What: Improved type hints
How to test the changes?
(Select all options that apply)
License