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

✨ Add Sample Concept #683

Merged
merged 6 commits into from
May 30, 2024
Merged

✨ Add Sample Concept #683

merged 6 commits into from
May 30, 2024

Conversation

chris-s-friedman
Copy link
Contributor

@chris-s-friedman chris-s-friedman commented May 22, 2024

Add Concept for Sample

This PR adds the concept for Sample. The approach for adding sample is to give it many of the same entities of biospecimen that make sense, such as shipping information, volume, etc and then make sample a parent to biospecimen, to indicate the relationship between biospecimen and sample. In the kf target api config, biospecimen and sample may be used interchangeable with preference given towards the sample entry.

Documentation for concepts is also added to the documentation site.

@chris-s-friedman chris-s-friedman changed the title Use Biospecimen Group Add Sample Concept May 29, 2024
@chris-s-friedman chris-s-friedman changed the title Add Sample Concept ✨ Add Sample Concept May 29, 2024
@chris-s-friedman chris-s-friedman force-pushed the D3B-570_biospecimen_group branch 3 times, most recently from e2a6460 to 18250dd Compare May 29, 2024 17:36
@chris-s-friedman chris-s-friedman marked this pull request as ready for review May 29, 2024 18:31
@chris-s-friedman chris-s-friedman requested a review from a team as a code owner May 29, 2024 18:31
@chris-s-friedman chris-s-friedman marked this pull request as draft May 29, 2024 18:31
@chris-s-friedman chris-s-friedman marked this pull request as ready for review May 29, 2024 18:34
Copy link
Member

@znatty22 znatty22 left a comment

Choose a reason for hiding this comment

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

Looking really good! Just minor comments on the docs

docs/source/design/concepts/index.rst Outdated Show resolved Hide resolved
docs/source/design/concepts/samples_and_specimens.rst Outdated Show resolved Hide resolved
docs/source/design/concepts/samples_and_specimens.rst Outdated Show resolved Hide resolved
docs/source/design/concepts/samples_and_specimens.rst Outdated Show resolved Hide resolved
docs/source/design/concepts/samples_and_specimens.rst Outdated Show resolved Hide resolved
docs/source/design/concepts/samples_and_specimens.rst Outdated Show resolved Hide resolved
@@ -429,6 +429,79 @@ def submit(cls, host, body):
return submit(host, cls, body)


class Sample:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should add a class docstring that explains BIOSPECIMEN_GROUP was a previously existing concept that has been used in ingest packages and that's why some fields (external_id) in the Biospecimen and/or Sample class first extract from SAMPLE and then secondarily extract from BIOSPECIMEN_GROUP

@znatty22
Copy link
Member

Tests might be failing bc you also have to add Sample right below Outcome:

@chris-s-friedman chris-s-friedman force-pushed the D3B-570_biospecimen_group branch 2 times, most recently from 66f6fe1 to 23054cb Compare May 30, 2024 15:58
@chris-s-friedman
Copy link
Contributor Author

Tests might be failing bc you also have to add Sample right below Outcome:

do you mean in all_targets? it's there :)

Copy link
Member

@znatty22 znatty22 left a comment

Choose a reason for hiding this comment

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

Looks good! Ok so looks like I was wrong before. Tests are failing bc the Sample class needs the following method definition like the other classes:

    @classmethod
    def submit(cls, host, body):
        return submit(host, cls, body)

🚨 Don't mess up formmatting

🚨 Don't mess up formmatting

point to sample when building sample key concept

🐛 Add submit function to sample
🚧 Working on documentation for samples and biospecimens

📝 Document samples and specimens

🔥 Remove test text

🚨 Enable black linting

📝 re-word docs for better clarity

Co-authored-by: Natasha Singh <[email protected]>

Update docs/source/design/concepts/samples_and_specimens.rst

Co-authored-by: Natasha Singh <[email protected]>

Update docs/source/design/concepts/samples_and_specimens.rst

Co-authored-by: Natasha Singh <[email protected]>

Update docs/source/design/concepts/samples_and_specimens.rst

Co-authored-by: Natasha Singh <[email protected]>

Update docs/source/design/concepts/samples_and_specimens.rst

Co-authored-by: Natasha Singh <[email protected]>

Update docs/source/design/concepts/samples_and_specimens.rst

Co-authored-by: Natasha Singh <[email protected]>

Update docs/source/design/concepts/index.rst

Co-authored-by: Natasha Singh <[email protected]>

🚨 Keep docs under 80 character line length

🚨 Documentation line length
@chris-s-friedman chris-s-friedman merged commit 026ed61 into master May 30, 2024
6 checks passed
@chris-s-friedman chris-s-friedman deleted the D3B-570_biospecimen_group branch May 30, 2024 19:37
chris-s-friedman added a commit that referenced this pull request May 31, 2024
## Release 1.21.0

### Summary

- Emojis: ✨ x2, ? x2
- Categories: Additions x2, Other Changes x2

### New features and changes

- [#686](#686) - ✨ D3B-586 sample relationship - [7cb601c](7cb601c) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#683](#683) - ✨ Add Sample Concept - [026ed61](026ed61) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#684](#684) -  🔧 add ucsf center - [469d097](469d097) by [HuangXiaoyan0106](https://github.com/HuangXiaoyan0106)
- [#680](#680) -  📝 add Variantyx sequencer - [8ccf9ec](8ccf9ec) by [HuangXiaoyan0106](https://github.com/HuangXiaoyan0106)
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