Skip to content
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

Closed
wants to merge 1 commit into from

Conversation

dakariakin
Copy link

@dakariakin dakariakin commented Nov 23, 2021

Why: common-workflow-lab#137
What: Improved type hints

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

Copy link
Member

@mvdbeek mvdbeek left a 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).

Comment on lines +332 to +333
update_time: datetime
create_time: datetime
Copy link
Member

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
Copy link
Member

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

Comment on lines +389 to +390
id: int
tool_id: int
Copy link
Member

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

@dakariakin dakariakin changed the title Added types to lib/galaxy/model/__init__.py WIP (wait for sqlalchemy2-stubs): Added types to lib/galaxy/model/__init__.py Nov 24, 2021
@dakariakin dakariakin marked this pull request as draft November 24, 2021 13:15
@dakariakin
Copy link
Author

@mvdbeek Thank you, that makes sense. I'll update this PR after sqlalchemy2-stubs installed

@mvdbeek mvdbeek modified the milestones: 22.01, 22.05 Jan 14, 2022
@mvdbeek mvdbeek removed this from the 22.05 milestone May 25, 2022
@dakariakin
Copy link
Author

The original issue is closed common-workflow-lab#137

@dakariakin dakariakin closed this Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants