Standardization of names in meta.yml for all tools & Big refactoring for all modules to subfolders#551
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors and standardizes module names and process identifiers across MSstats and DIA-NN modules to the {TOOL}_{PROCESS} naming convention.
- Renamed MSstats TMT and LFQ modules in metadata and process definitions.
- Updated various DIA-NN process blocks to include
DIANN_prefix and underscore. - Adjusted module metadata for DIANN convert to match naming style.
Reviewed Changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| modules/local/msstats/msstats_tmt/meta.yml | Updated module name to MSSTATS_TMT |
| modules/local/msstats/msstats_tmt/main.nf | Renamed process to MSSTATS_TMT |
| modules/local/msstats/msstats_lfq/meta.yml | Updated module name to MSSTATS_LFQ |
| modules/local/msstats/msstats_lfq/main.nf | Renamed process to MSSTATS_LFQ |
| modules/local/diann/summary/main.nf | Renamed process to DIANN_SUMMARY |
| modules/local/diann/insilico_library_generation/main.nf | Renamed process to DIANN_INSILICO_LIBRARY_GENERATION |
| modules/local/diann/generate_cfg/main.nf | Renamed process to DIANN_GENERATE_CFG |
| modules/local/diann/convert/meta.yml | Changed module name to diannconvert |
| modules/local/diann/convert/main.nf | Renamed process to DIANN_CONVERT |
| modules/local/diann/assemble_empirical_library/main.nf | Renamed process to DIANN_ASSEMBLE_EMPIRICAL_LIBRARY |
Comments suppressed due to low confidence (1)
modules/local/diann/convert/meta.yml:1
- Module name should follow the {TOOL}_{PROCESS} convention and match its process identifier. Consider renaming this to
DIANN_CONVERTfor consistency.
name: diannconvert
There was a problem hiding this comment.
Pull Request Overview
This PR refactors and standardizes module names and process identifiers across the project.
- Updated msstats module names and process identifiers to include descriptive suffixes (TMT, LFQ).
- Revised DIANN module process names to use a consistent DIANN_ prefix and underscore-separated naming.
- Adjusted meta.yml files to align with the standardized process names.
Reviewed Changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| modules/local/msstats/msstats_tmt/meta.yml | Updated module name from MSSTATSTMT to MSSTATS_TMT |
| modules/local/msstats/msstats_tmt/main.nf | Updated process name from MSSTATSTMT to MSSTATS_TMT |
| modules/local/msstats/msstats_lfq/meta.yml | Updated module name from MSSTATS to MSSTATS_LFQ |
| modules/local/msstats/msstats_lfq/main.nf | Updated process name from MSSTATS to MSSTATS_LFQ |
| modules/local/diann/summary/main.nf | Updated process name from DIANNSUMMARY to DIANN_SUMMARY |
| modules/local/diann/insilico_library_generation/main.nf | Updated process name from SILICOLIBRARYGENERATION to DIANN_INSILICO_LIBRARY_GENERATION |
| modules/local/diann/generate_cfg/main.nf | Updated process name from GENERATE_DIANN_CFG to DIANN_GENERATE_CFG |
| modules/local/diann/convert/meta.yml | Updated module name from DIANNCONVERT to diannconvert (note casing inconsistency) |
| modules/local/diann/convert/main.nf | Updated process name from DIANNCONVERT to DIANN_CONVERT |
| modules/local/diann/assemble_empirical_library/main.nf | Updated process name from ASSEMBLE_EMPIRICAL_LIBRARY to DIANN_ASSEMBLE_EMPIRICAL_LIBRARY |
Comments suppressed due to low confidence (1)
modules/local/diann/convert/meta.yml:1
- The module name 'diannconvert' is in lowercase while other similar module names use uppercase with underscores (e.g., DIANN_CONVERT). Consider updating it for consistency.
name: diannconvert
|
Thanks to all for the feedback, here is the current As mentioned and suggested by @jpfeuffer the structure is the following:
As suggested by @jpfeuffer, a different config was created in case the user wants to output all the intermediate steps Still, I added some structure to each folder like: Please provide feedback before merging this PR. Additionally, I organised the modules config similar to other nf-core pipelines. |
There was a problem hiding this comment.
Pull Request Overview
This PR standardizes naming conventions and refactors module organization for consistency across the project. Key changes include:
- Renaming process names (e.g. from DIANNCONVERT to CONVERT_RESULTS in main.nf and updating meta.yml names accordingly).
- Restructuring modules into subfolders based on their framework (openms, diann, msstats, utils) and aligning naming across meta and main files.
- Updating configuration files (modules.config, verbose_modules.config, .nf-core.yml) to reflect the new naming standards and publishing paths.
Reviewed Changes
Copilot reviewed 123 out of 123 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| modules/local/diann/convert_results/meta.yml | Updated the module name from DIANNCONVERT to convert_results to align with naming standards. |
| modules/local/diann/convert_results/main.nf | Renamed the process from DIANNCONVERT to CONVERT_RESULTS for consistency. |
| conf/modules/verbose_modules.config | Adjusted publishDir settings to support standardized names; changes appear consistent. |
| conf/modules/modules.config | Updated withName regexes and process names to match the new naming scheme for improved clarity. |
| conf/igenomes_ignored.config | Removed as part of the standardization process. |
| CHANGELOG.md | Version updated from 1.4.1 to 1.5.0dev with corresponding changelog updates. |
| .nf-core.yml | Updated lint configuration and version, though duplicate config entries were introduced. |
Comments suppressed due to low confidence (1)
.nf-core.yml:20
- Duplicate entries for configuration files 'conf/modules.config' and 'conf/igenomes_ignored.config' are present in the lint configuration. Please remove the duplicates to avoid redundancy and potential configuration conflicts.
- - conf/modules.config
There was a problem hiding this comment.
Pull Request Overview
This PR standardizes the naming conventions for modules and processes across the codebase and refactors modules into dedicated subfolders. It ensures that the names in meta.yml are in lower-case and the process names in main.nf follow the UPERCASE style, while also updating configuration files for publishing directories and removing legacy configuration files.
- Standardized module and process names (e.g., DIANNCONVERT → convert_results / CONVERT_RESULTS)
- Updated configuration files (conf/modules/verbose_modules.config and conf/modules/modules.config) to support new naming conventions and publishing paths
- Removed legacy iGenomes configuration and updated version numbers in CHANGELOG.md and .nf-core.yml
Reviewed Changes
Copilot reviewed 123 out of 123 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/local/diann/convert_results/meta.yml | Renamed module identifier to lower-case to match naming standards |
| modules/local/diann/convert_results/main.nf | Updated process name to reflect standardized naming conventions |
| conf/modules/verbose_modules.config | Updated publish directory paths for various processes |
| conf/modules/modules.config | Revised process name patterns and publish directory settings for consistency |
| conf/igenomes_ignored.config | Removed legacy iGenomes configuration |
| CHANGELOG.md | Bumped version to account for standardized naming changes |
| .nf-core.yml | Updated versions and configuration file references to align with recent changes |
Comments suppressed due to low confidence (1)
modules/local/diann/convert_results/meta.yml:1
- The module name has been updated to 'convert_results' in meta.yml. Please ensure that all references to this module in related documentation and other configuration files are updated accordingly.
-name: DIANNCONVERT
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Looks good!! What is an example for DIA? |
@jpfeuffer, The extended DIA: |
|
LGTM |
|
results_iso doesn’t have spectra folder? Did not see it in the tree above |
@timosachsenberg it will be only available if |
enryH
left a comment
There was a problem hiding this comment.
I like the easier to read module names and the new module structure (plus test and configs).
Renaming and moving files make it a bit hard to evaluate that the moved nexflow processes in modules are all correct (deleted + added file with changes -> often differences are not highlighted). So separating renaming and moving would help if we do this ever again.
See my few comments.
Co-authored-by: Henry Webel <heweb@dtu.dk>
Co-authored-by: Henry Webel <heweb@dtu.dk>
Co-authored-by: Henry Webel <heweb@dtu.dk>
A little bit of discussion about this PR from @ypriverol. Currently, quantms has evolved with no standarization of the naming system for modules, tools, subworkflows, workflows etc. These are the main issues I have found:
modules/local/openms/{}, but the ones from diann are in the parent localmodules/local/{}🤔.DIANNCONVERTand others likePREPROCESS_EXPDESIGN. This is difficult to follow because we don't know why is that and how the decision was made.main.nfcompared with themeta.yml. For example, you can be calledPREPROCESS_EXPDESIGNin themain.nfand in the meta.ymlpreprocessexpdesignThis PR is the first iteration standardising the names, modules, folders and import style. These are some of the decisions I have made:
_to split between words, this is following nf-core and Python style.COMETin favor of using multiple decorative things like SEARCH_ENGINE_COMET. In some cases is not possible, like INFERENCE and EPYFANY, then I have to put a decoration because epifany is an inference tool. Then I use protein_inference_epifany and protein_inference_genericpsm_rescoring, inside it has a main.nf.Please feel free to give me feedback here and also in the way we are naming the folders #551 (comment). This will be really impactful for users, then all comments are welcome.