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

refactor: update code for type checks #169

Merged
merged 3 commits into from
May 18, 2024
Merged

refactor: update code for type checks #169

merged 3 commits into from
May 18, 2024

Conversation

JaeAeich
Copy link
Contributor

IMPORTANT: Please create an issue before filing a pull request! Changes need to be discussed before proceeding. Please read the contribution guidelines.

Details

This PR aims to add type-checking, partly addressing #116

@JaeAeich JaeAeich marked this pull request as draft February 10, 2024 16:01
@JaeAeich JaeAeich marked this pull request as ready for review February 10, 2024 16:22
pro_tes/ga4gh/tes/models.py Outdated Show resolved Hide resolved
pro_tes/ga4gh/tes/models.py Outdated Show resolved Hide resolved
pro_tes/ga4gh/tes/task_runs.py Outdated Show resolved Hide resolved
pro_tes/ga4gh/tes/task_runs.py Outdated Show resolved Hide resolved
pro_tes/ga4gh/tes/task_runs.py Outdated Show resolved Hide resolved
@JaeAeich
Copy link
Contributor Author

JaeAeich commented May 4, 2024

I'm not really sure about,

task_id = logs[0].metadata["remote_task_id"]

so ignored it atm, the error it gives is

pro_tes/ga4gh/tes/task_runs.py:391: error: Value of type "Metadata" is not indexable  [index]

which makes sense looking at the class definition of it, maybe I'm missing something 🤷‍♂️ !

PS: This is solved.

@JaeAeich JaeAeich requested a review from uniqueg May 4, 2024 05:22
@uniqueg
Copy link
Member

uniqueg commented May 18, 2024

I'm not really sure about,

task_id = logs[0].metadata["remote_task_id"]

so ignored it atm, the error it gives is

pro_tes/ga4gh/tes/task_runs.py:391: error: Value of type "Metadata" is not indexable  [index]

which makes sense looking at the class definition of it, maybe I'm missing something 🤷‍♂️ !

PS: This is solved.

I guess this is where .metadata gets overridden, against the model definition. Not good. But I guess this code needs a good round of refactoring anyway.

Will leave this as is, with the type: ignore.

@uniqueg uniqueg changed the title feat(ci): add mypy type checking refactor: update code for type checks May 18, 2024
@uniqueg uniqueg merged commit 4198f71 into dev May 18, 2024
5 checks passed
@uniqueg uniqueg deleted the types branch May 18, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants