Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

Version 1.2 [PLEASE DO NOT MERGE YET] #120

Merged
merged 260 commits into from Nov 2, 2023
Merged

Version 1.2 [PLEASE DO NOT MERGE YET] #120

merged 260 commits into from Nov 2, 2023

Conversation

ypriverol
Copy link
Member

@ypriverol ypriverol commented Oct 19, 2023

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/quantms branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

jspaezp and others added 30 commits August 3, 2023 18:02
…es and fixed mass calc

Experimental/feature/bruker data
* added report info
* split decompression step
* Included ms1 TIC/BPC
* added new data to report channel
* added convert_dotd to the schema
* fixed bug where passed mass accuracies were bypassed
* code formatting
@fabianegli
Copy link
Contributor

So after the changes in #127 mypy is happy and we fixed a bug.

% mypy --ignore-missing-imports bin/mzml_statistics.py
Success: no issues found in 1 source file

fabianegli and others added 13 commits November 1, 2023 14:53
Co-authored-by: Fabian Egli <fabianegli@users.noreply.github.com>
As it is, there is too much logic in this class for it to be a dataclass.
The only_first argument was never used in any function call in the script. It is also very easy to get the whole list of files matching a glob, if necessary.
and make validate_diann_version a method
@fabianegli
Copy link
Contributor

I am still a little bit dismayed by the general lack of tests for the Python scripts, but after #128 is merged, I will approve the PR.

@fabianegli
Copy link
Contributor

fabianegli commented Nov 1, 2023

I introduced a bug in the diann_convert in #128 and the PR got merged before I saw it. #129 should fix it again.

@fabianegli fabianegli self-requested a review November 1, 2023 20:20
Copy link
Contributor

@fabianegli fabianegli left a comment

Choose a reason for hiding this comment

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

I really like all the added capabilities of the pipeline and the various improvements! Thank you everyone for your work.

For the future, I think we should find a way to run tests for the Python scripts. Having tests for them would make development on them much faster, safer and enjoyable.

@fabianegli fabianegli merged commit fa34d79 into master Nov 2, 2023
35 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants