Skip to content

sync with root-inheritance-fix from VRS#441

Merged
larrybabb merged 6 commits intomainfrom
vrs-root-inheritance-fix
Aug 12, 2024
Merged

sync with root-inheritance-fix from VRS#441
larrybabb merged 6 commits intomainfrom
vrs-root-inheritance-fix

Conversation

@larrybabb
Copy link
Copy Markdown
Contributor

removed reference to VRS ValueObject which was removed in VRS branch root-inheritance-fix PR under review.

@korikuzma please check my work and convert let me know how to get this completed and released.

Comment thread src/ga4gh/vrs/models.py


class _ValueObject(DomainEntity, ABC):
class _ValueObject(Entity, ABC):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is fine, but to be consistent with VRS I wonder if we should remove _ValueObject and just extend Entity?

@larrybabb
Copy link
Copy Markdown
Contributor Author

@korikuzma I need some pointers on how to update the pypi stuff (I think) so that the notebooks can be updated to address the fix to the efo: -> EFO: PR that was made to vrs 2.x recently. I'd like to know how to do that if you have the time to show me.

@larrybabb larrybabb marked this pull request as ready for review August 12, 2024 01:36
@larrybabb larrybabb requested review from a team as code owners August 12, 2024 01:36
@korikuzma
Copy link
Copy Markdown
Contributor

@larrybabb Since we no longer use python jsonschema objects, we have to manually update the source code in VRS-Python. For the notebooks, there are some hard-coded EFOs which would also need to be updated manually.

@korikuzma
Copy link
Copy Markdown
Contributor

@larrybabb why was EFO case changed to lowercase in this commit? ga4gh/vrs@cafe826

@larrybabb larrybabb requested a review from korikuzma August 12, 2024 18:58
Copy link
Copy Markdown
Contributor

@korikuzma korikuzma left a comment

Choose a reason for hiding this comment

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

Either update notebooks in this PR or separate

@larrybabb larrybabb merged commit e195188 into main Aug 12, 2024
@larrybabb larrybabb deleted the vrs-root-inheritance-fix branch August 12, 2024 20:07
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