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 tissue type when creating scout config file #502

Closed
wants to merge 42 commits into from

Conversation

northwestwitch
Copy link
Member

@northwestwitch northwestwitch commented Dec 10, 2019

This PR should add a field tissue_type for each individual when creating a scout config file.
fix #497

How to prepare for test:

  • install on hasta stage bash /home/proj/production/servers/resources/hasta.scilifelab.se/update-cg-stage.sh pass_tissue_type
  • activate stage: us

How to test:

  • cg upload scout bosscod

Expected test outcome:

  • The load command should create a yaml config file containing a tissue type set to 'unknown' for the case sample. (this happens because the sample is not in lims stage database, but in the lims one). Maybe there is a better case to test the command with?

Review:

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

This is a minor version update because it adds a new functionality.

@coveralls
Copy link
Collaborator

coveralls commented Dec 10, 2019

Coverage Status

Coverage increased (+0.02%) to 64.902% when pulling 938fd99 on pass_tissue_type into 4d5df8e on master.

@northwestwitch
Copy link
Member Author

I'm sure it's because I don't have much experience with it, but it's 2 days that I'm literally getting crazy over linting problems: the more I fix them, the more pop up at the next build.

Tried reformatting the file with black but I still get new errors every build. Any help would be very appreciated. I'm going to leave this PR for when I will have more time otherwise.

@barrystokman
Copy link
Contributor

See last comment on this issue: pylint-dev/pylint#2944, the indentation message might be caused by black

line 221, col 4: Refactor: [no-self-use]: MockLims.lims_samples: Method could be a function

    def lims_samples(self):
        lims_family = json.load(open("tests/fixtures/report/lims_family.json"))
        return lims_family["samples"]

This method/function doesn't touch the class at all so maybe use @staticmethod?

@northwestwitch
Copy link
Member Author

See last comment on this issue: PyCQA/pylint#2944, the indentation message might be caused by black

Very likely, the error showed immediately after I reformatted with black, I'll try to revert to a version before. Thanks!

@northwestwitch
Copy link
Member Author

northwestwitch commented Dec 11, 2019

Thanks @hassanfa, the 4 spaces messages are gone now!

@northwestwitch
Copy link
Member Author

YESSS IT PASSED!!!
Thank you so much @barrystokman and @hassanfa!
I might start crying now (happy tears), so good thing I'm working from home so I'll spare you the scene!! XD

@hassanfa
Copy link
Contributor

@northwestwitch worthy of adding to resume 🤣

@northwestwitch
Copy link
Member Author

@northwestwitch worthy of adding to resume 🤣

Indeed! :D

@northwestwitch
Copy link
Member Author

This is ready for review, anybody?

@patrikgrenfeldt
Copy link
Contributor

I have reviewed the code, looked good, seems like there is some conflicts that needs relolution:
image

@northwestwitch
Copy link
Member Author

Thanks @patrikgrenfeldt, I'll fix them asap!

@northwestwitch
Copy link
Member Author

I worked on this PR a month ago and many things have changed since then, I'll try to fix it here but it might take longer than I prevented..!

@northwestwitch
Copy link
Member Author

Closing this and opening a new PR with the same code from the updated master. Otherwise it was conflict hell!

@Mropat Mropat deleted the pass_tissue_type branch August 4, 2022 20:02
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.

Tissue type should be passed on from lims (is it there?) to Scout.
5 participants