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.

Sign up for GitHub

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

Jump to bottom

feat!: replace instance var lists with pydantic model #374

Merged
merged 7 commits into from
Jul 9, 2024
Merged

feat!: replace instance var lists with pydantic model #374

merged 7 commits into from
Jul 9, 2024

Conversation

korikuzma
Copy link
Member

I think this is still a wip but @jsstevenson wanted to make you aware

korikuzma added 4 commits June 25, 2024 08:30
@korikuzma
refactor!: remove extract_harvester from transform method
6a063c3 
close #341

* create new method `extract_harvester` to separate these out
* `transform` now accepts harvested data as a parameter
@korikuzma
rename extract_harvester to extract_harvested_data
4a80b22
@korikuzma
refactor!: rename Transform instance variables
e5b734a 
close #253
* molecular_profiles -> categorical_variations
* therapeutics -> therapeutic_procedures
* diseases -> conditions
@korikuzma
feat!: replace instance var lists with pydantic model
ddaddb6
@korikuzma korikuzma added priority:low Low priority technical debt A feature/requirement implemented in a sub-optimal way & must be re-written. Contrast to "cleanup" labels Jun 28, 2024
Base automatically changed from issue-253 to staging July 1, 2024 14:39
@korikuzma
Copy link
Member Author

I think refactoring cache strategy should be a separate issue.

@korikuzma korikuzma added priority:medium Medium priority and removed priority:low Low priority labels Jul 8, 2024
@korikuzma korikuzma requested a review from jsstevenson July 8, 2024 13:50
@korikuzma korikuzma marked this pull request as ready for review July 8, 2024 13:50
@korikuzma korikuzma self-assigned this Jul 9, 2024
jsstevenson
jsstevenson approved these changes Jul 9, 2024
View reviewed changes
Copy link
Member

@jsstevenson jsstevenson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 great. I think it makes a lot of sense to avoid using dicts as an interface layer when we could be using namedtuples (gives some field definitions, enables autocomplete), dataclasses (namedtuples but also enables type annotation) or pydantic models (dataclasses but with runtime validation).

@korikuzma korikuzma merged commit e68763e into staging Jul 9, 2024
13 checks passed
@korikuzma korikuzma deleted the pydantic branch July 9, 2024 12:49
@jsstevenson jsstevenson mentioned this pull request Jan 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsstevenson jsstevenson jsstevenson approved these changes

Assignees

@korikuzma korikuzma

Labels
priority:medium Medium priority technical debt A feature/requirement implemented in a sub-optimal way & must be re-written. Contrast to "cleanup"
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@korikuzma @jsstevenson