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

fetch tissue type from original sample #3979

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rannick
Copy link
Contributor

@rannick rannick commented Nov 27, 2024

Description

Added

Changed

Fixed

How to prepare for test

  • Ssh to relevant server (depending on type of change)
  • Use stage: us
  • Paxa the environment: paxa
  • Install on stage (example for Hasta):
    bash /home/proj/production/servers/resources/hasta.scilifelab.se/update-tool-stage.sh -e S_cg -t cg -b [THIS-BRANCH-NAME] -a

How to test

  • Do ...

Expected test outcome

  • Check that ...
  • Take a screenshot and attach or copy/paste the output.

Review

  • Tests executed by
  • "Merge and deploy" approved by
    Thanks for filling in who performed the code review and the test!

This version is a

  • MAJOR - when you make incompatible API changes
  • MINOR - when you add functionality in a backwards compatible manner
  • PATCH - when you make backwards compatible bug fixes or documentation/instructions

Implementation Plan

  • Document in ...
  • Deploy this branch on ...
  • Inform to ...

@rannick rannick requested a review from a team as a code owner November 27, 2024 12:06
Copy link
Contributor

@henrikstranneheim henrikstranneheim left a comment

Choose a reason for hiding this comment

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

Great!

@@ -345,6 +345,16 @@ def get_sample_ids_by_case_id(self, case_id: str = None) -> Iterator[str]:
for link in case.links:
yield link.sample.internal_id

def get_original_sample_ids_by_case_id(self, case_id: str = None) -> Iterator[str]:
"""Return original sample ids (go to original sample id for downloaded samples) from case id."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""Return original sample ids (go to original sample id for downloaded samples) from case id."""
"""Return original sample id from case id."""

@@ -345,6 +345,16 @@ def get_sample_ids_by_case_id(self, case_id: str = None) -> Iterator[str]:
for link in case.links:
yield link.sample.internal_id

def get_original_sample_ids_by_case_id(self, case_id: str = None) -> Iterator[str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

How about a unit test for this function?

Comment on lines +348 to +356
def get_original_sample_ids_by_case_id(self, case_id: str = None) -> Iterator[str]:
"""Return original sample ids (go to original sample id for downloaded samples) from case id."""
case: Case = self.get_case_by_internal_id(internal_id=case_id)
self._is_case_found(case=case, case_id=case_id)
for link in case.links:
if link.sample.from_sample:
original_sample: Sample = link.sample.from_sample
yield original_sample.internal_id
yield link.sample.internal_id
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we require.a yield here as we only fetch one id it seems?

Suggested change
def get_original_sample_ids_by_case_id(self, case_id: str = None) -> Iterator[str]:
"""Return original sample ids (go to original sample id for downloaded samples) from case id."""
case: Case = self.get_case_by_internal_id(internal_id=case_id)
self._is_case_found(case=case, case_id=case_id)
for link in case.links:
if link.sample.from_sample:
original_sample: Sample = link.sample.from_sample
yield original_sample.internal_id
yield link.sample.internal_id
def get_original_sample_ids_by_case_id(self, case_id: str = None) -> str:
"""Return original sample ids (go to original sample id for downloaded samples) from case id."""
case: Case = self.get_case_by_internal_id(internal_id=case_id)
self._is_case_found(case=case, case_id=case_id)
for link in case.links:
if link.sample.from_sample:
original_sample: Sample = link.sample.from_sample
return original_sample.internal_id
return link.sample.internal_id

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.

3 participants