Trending

[mayabrandi]

This PR adds two commands to the cli to generate genotype data in dict format:

• genotype prepare-analysis Gets analysis data for sample/samples from the genotype and analysis tables
  • genotype prepare-sample Gets data for sample/samples from the sample table

Each command has the two options:

• -s, --sample-id TEXT return sample with specific sample id.
• -d, --days TEXT return samples added a specific number of days ago.

How to test:

1. install genotype on hasta stage
2. Try the different commands and options:

The following will return a dicts with samples created within the last 150 days.

genotype --database prepare-sample --days 150 Expected outcome: {"ACC5218A10": {"_id": "ACC5218A10", "status": null, "sample_created_in_genotype_db": "2019-02-08", "sex": "male", "comment": null}, "ACC5218A8": {"_id": "ACC5218A8", ... }

genotype --database prepare-analysis --days 150 Expected outcome: {"ACC5218A10": {"_id": "ACC5218A10", "snps": {"sequence": {"rs10144418": ["T", "C"],... "ACC5346A3": {"_id":"ACC5346A3",... }

Try also the -s option with eg sample ACC5346A3.

Review:

• [x] code approved by @patrikgrenfeldt
• [x] tests executed by @mayabrandi 9/12
• [x] "Merge and deploy" approved by @patrikgrenfeldt and @emiliaol Thanks for filling in who performed the code review and the test!

Since this is adding a new feature to the package and is not breaking any old stuff, it should get a minor version bump. :)

[patrikgrenfeldt]

Yes, I'd like the applications to not know about each other. In other words, they should preferably have no names from each other

i.e. name genotype/store/vogue.py should be abstracted to what is does or themes

That is what I suggest, what do you say?

[mayabrandi]

Can this be merged now or do you have any further comments? @patrikgrenfeldt I don't manage to fix the lint issues that are left.

[ingkebil]

I looked at the two remaining linting issues and they complain about a unused passed variable. Any reason you can't just remove that variable?

[ingkebil]

I've updated the PR text. Can you fill in how to tests this, maybe even test it already to show that it works, and motivate the version bump?

[mayabrandi]

Hej! Yes. If I remove the variable the database queries inside the functions build_snp_dict and prepare_trending give connection errors, eventhougn the genotype_db is never used explicitly. I can show you if you have time. Maybe you can come with a better solution.

9 sep. 2019 kl. 16:28 skrev Kenny Billiau notifications@github.com:

I looked at the two remaining linting issues and they complain about a unused passed variable. Any reason you can't just remove that variable?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Clinical-Genomics/genotype/pull/28?email_source=notifications&email_token=AAJ65XJJLVJTNDISRLIJFYTQIZMQHA5CNFSM4INWHJPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6HY6II#issuecomment-529501985, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJ65XPFNBFL252RZHJWH3LQIZMQHANCNFSM4INWHJPA.

[mayabrandi]

Will do!

9 sep. 2019 kl. 16:31 skrev Kenny Billiau notifications@github.com:

I've updated the PR text. Can you fill in how to tests this, maybe even test it already to show that it works, and motivate the version bump?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Clinical-Genomics/genotype/pull/28?email_source=notifications&email_token=AAJ65XJ576O3VPE722ADCRDQIZM27A5CNFSM4INWHJPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6HZJYY#issuecomment-529503459, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJ65XLICAW3QFOP33BL5QLQIZM27ANCNFSM4INWHJPA.

[ingkebil]

Alright, I think we can let the last two linting errors just slip by. When the test cases are documented, I can sign off for deploy

[mayabrandi]

I have updated the test spec in the PR text now. Se if it is okej.

10 sep. 2019 kl. 10:04 skrev Kenny Billiau notifications@github.com:

Alright, I think we can let the last two linting errors just slip by. When the test cases are documented, I can sign off for deploy

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Clinical-Genomics/genotype/pull/28?email_source=notifications&email_token=AAJ65XPWVV4M7PUINMWFIELQI5IIHA5CNFSM4INWHJPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6KHBZY#issuecomment-529821927, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJ65XLE5SMMOSADG4DUPX3QI5IIHANCNFSM4INWHJPA.

[ingkebil]

There is no update script for genotype-stage on clinical-db, let me quickly make one.

I would change the text to the following so it is clear for whomever that tests it what to do. It also configures genotype

How to test:

• install genotype on clinical-db stage
• In stage, run command: genotype prepare-trending --days 150

[mayabrandi]

Thanks a lot!

10 sep. 2019 kl. 11:03 skrev Kenny Billiau notifications@github.com:

There is no update script for genotype-stage on clinical-db, let me quickly make one.

I would change the text to the following so it is clear for whomever that tests it what to do. It also configures genotype

How to test:

install genotype on clinical-db stage In stage, run command: genotype --database prepare-trending --days 150 This will return a dict with samples created within the last 150 days. Keys are sample ids and values are documents for the trending database.

One can also run the command: genotype --database prepare-trending --sample ACC5346A3

This will return the document for that sample .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Clinical-Genomics/genotype/pull/28?email_source=notifications&email_token=AAJ65XP7BYL7VS5ETDIRXNDQI5PG7A5CNFSM4INWHJPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6KMORA#issuecomment-529844036, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJ65XPJDWN6P6X2TG4UOA3QI5PG7ANCNFSM4INWHJPA.

[ingkebil]

Oh wait, I'm wrong, this is to be tested on hasta, right? It needs to be coupled to cg, right?

[mayabrandi]

Test has been performed on hasta with expected results.

[ingkebil]

Don't forget to motivate the version bump

[patrikgrenfeldt]

"Since this is adding a new feature to the package, it should probably get a major version bump?" @mayabrandi you may add to an API and calling it minor as long as it doesn't break any old parts

[mayabrandi]

Okej!

24 okt. 2019 kl. 10:06 skrev patrikgrenfeldt notifications@github.com:

"Since this is adding a new feature to the package, it should probably get a major version bump?" @mayabrandi https://github.com/mayabrandi you may add to an API and calling it minor as long as it doesn't break any old parts

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Clinical-Genomics/genotype/pull/28?email_source=notifications&email_token=AAJ65XNKTI3YYLUMK633SZLQQFJSFA5CNFSM4INWHJPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECEDWCQ#issuecomment-545798922, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ65XIPKQTGLIMTL4Q7RETQQFJSFANCNFSM4INWHJPA.

[mayabrandi]

@patrikgrenfeldt @henrikstranneheim Ive split the cli into two different commands for the two different types of queries to the database, as we talked about. Also tried to take away anything that reminds of mongo in the doc strings, variables and function names. Pleas have a look and let me know whats left. When we have decided that this is what the structure will look like, I will update the tests. They are out of date atm. Thanks for today!

[mayabrandi]

@patrikgrenfeldt @henrikstranneheim Ive updated the things we talked about here. remove _id from export dicts remove sample option rename from trending to export remove export of empty dict I did not change from option to argument. had some issues there

[mayabrandi]

@patrikgrenfeldt @henrikstranneheim I consider myself done with the tests now, if you want to have a look.

[mayabrandi]

Is this one ready for approval? The failing checks don't seem to have anything to do with this PR. @patrikgrenfeldt

[ingkebil]

The tests have been run in October and several dozen new commits have been added. Can you rerun the tests?

[patrikgrenfeldt]

The checks work again, https://travis-ci.org/Clinical-Genomics/genotype/builds/621520029?utm_source=github_status&utm_medium=notification, there are some linting left

[mayabrandi]

Can this one be approved and merged now @patrikgrenfeldt?

[mayabrandi]

[patrikgrenfeldt]

deployed