feat: Improve datavzrd tables #93
Conversation
There was a problem hiding this comment.
general: get rid of commented out code, tidy scripts (double empty lines after comments), ensure that scripts are named after what they actually do. Bad example: copy_study_items_sig_terms. The script seems to do more, it is unclear on what kind of data it operates. Sometimes, less is more, e.g. postprocess_go_enrichment.py.
|
Just wanted to give you a heads-up that I am planning to merge another pull request tomorrow, which will change the datavzrd templates and update to the latest version of the datavzrd wrapper: #92 If you need me to look into updating this branch after the merge, let me know. All the changes are still fresh in my mind, so I should be reasonably confident in resolving conflicts that might arise. |
|
So, I think I have reasonably resolved the conflicts with the latest main branch. Let's see if the tests pass -- if they don't, I'll try to resolve remaining issues. Afterwards, I think we should still:
|
workflow/resources/datavzrd/meta_comparison-diffexp-template.yaml
Outdated
Show resolved
Hide resolved
…-kallisto-sleuth into locotact_p18
Co-authored-by: David Laehnemann <[email protected]>
…-kallisto-sleuth into locotact_p18
…ining matrix, sort in the right order
There was a problem hiding this comment.
I have now edited most of David's comments. In general, I still have a problem understanding the review process, because I see a lot of outdated comments in the conversation tab and for every comment I write on an outdated comment a new box is opened elsewhere. I find this very confusing. Not all of your comments appear in the Files Changed tab. I have now gone through both tabs several times and hope I have not forgotten anything.
If I am generally acting incorrectly during the Git workflow, I would be happy to receive suggestions for improvement!
Comments that are still pending and need to be fixed before merging. I'll try to do that on Monday (and today):
Questions:
- workflow/resources/datavzrd/spia-template.yaml: How to show gene_ratio, should I use bar plot or circle_plots
- config/config.yaml: What comments are missing for meta_comparisons
ToDo:
- workflow/report/workflow.rst: Include "For sample metadata, see {{ snakemake.config["samples"] }}_." Maybe linkout to sample.tsv in the report.
- workflow/envs/pystats.yaml: Look which dependencies are really needed
- workflow/scripts/compare_diffexp.py.ipynb: Why is it so big? Are those artifacts?
- Let it run and create report to verify everything
workflow/resources/custom_vega_plots/circle_diagram_de_genes.json
Outdated
Show resolved
Hide resolved
|
Just had a quick look, and apart from the things you list, I think you caught all of the other comments. I think all this weirdness in where this appear or don't really comes from this whole So maybe you can go through this on Monday, until you are satisfied with the result. And then I will do a fresh round of review, that hopefully offers a less confusing experience for us both. Sorry for my part in causing the confusion here, but I think we're almost through... 😅 |
Co-authored-by: David Laehnemann <[email protected]>
Co-authored-by: David Laehnemann <[email protected]>
…-kallisto-sleuth into locotact_p18
There was a problem hiding this comment.
Fixed the open problems. For each of the big jupyter notebooks I created a new python file. The pystats env had really way too much dependencies in it. I removed the unnecessary ones. I have problems linking to the specific input samples file in the workflow.rst, so right now I am linking to the input folder containing the samples file.
…-kallisto-sleuth into locotact_p18
Apply Davids suggestions for better code quality Co-authored-by: David Laehnemann <[email protected]>
There was a problem hiding this comment.
This looks good to me! Just 3 little suggestions, one a typo, two of them more descriptive titles for the report (but which will only be used, once we have wildcards available in snakemake report() captions, see snakemake/snakemake#3021).
Feel free to merge, once you have incorporated the suggestions (or something similar). And ping me, in case you don't have the necessary permissions to merge.
dlaehnemann
dismissed
johanneskoester’s stale review
All requests have been addressed, no need to bother Johannes.
Co-authored-by: David Laehnemann <[email protected]>
Co-authored-by: David Laehnemann <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [2.7.0](v2.6.0...v2.7.0) (2024-08-15) ### Features * Improve datavzrd tables ([#93](#93)) ([93512b8](93512b8)) ### Bug Fixes * Fix missing output in spia.R when no significant genes are found ([#103](#103)) ([bc0d017](bc0d017)) * Handle missing bam columns in units.tsv ([#105](#105)) ([bae88d0](bae88d0)) * Remove non-existent outputs in spia rule ([#102](#102)) ([0fbb930](0fbb930)) * update to latest datavzrd ([417ec3b](417ec3b)) ### Performance Improvements * datavzrd wrapper `v3.12.1`, offer-excel configurable, free disk space for CI, dynamic sleuth_init mem_mb, pure download rules as localrules ([#92](#92)) ([70850fb](70850fb)) * Update datavzrd wrapper ([#98](#98)) ([e5eb0e0](e5eb0e0)) * Update samtools fast separate wrapper ([#100](#100)) ([65d8f41](65d8f41)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
No description provided.