Skip to content

Always prettify modules.json#4063

Merged
muffato merged 3 commits intodevfrom
prettier_modules_json
Mar 2, 2026
Merged

Always prettify modules.json#4063
muffato merged 3 commits intodevfrom
prettier_modules_json

Conversation

@muffato
Copy link
Copy Markdown
Member

@muffato muffato commented Feb 27, 2026

I got a bit tired of having to re-run prettier after nf-core commands because they change the layout of modules.json (especially around installed_by).

This is a patch to make prettier the default write mode for modules.json

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@muffato muffato self-assigned this Feb 27, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.43%. Comparing base (c804c1e) to head (a103084).
⚠️ Report is 18 commits behind head on dev.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@muffato muffato marked this pull request as ready for review February 27, 2026 10:57
Copilot AI review requested due to automatic review settings February 27, 2026 10:57
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes modules.json formatting consistent by defaulting ModulesJson.dump() to run Prettier, so that nf-core commands stop rewriting the file in a non-prettified layout (notably around installed_by).

Changes:

  • Change ModulesJson.dump() default to run_prettier=True.
  • Update call sites to rely on the new default (dump() instead of dump(run_prettier=True)).
  • Add a changelog entry documenting the behavior change.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
nf_core/modules/modules_json.py Switch default dump behavior to run Prettier and update internal usage.
nf_core/components/install.py Use the new ModulesJson.dump() default during component install.
CHANGELOG.md Document “Always prettify modules.json”.
Comments suppressed due to low confidence (1)

nf_core/modules/modules_json.py:1136

  • Changing ModulesJson.dump() default to run Prettier will now invoke pre-commit run prettier on every internal call site that previously relied on the default (eg. ModulesJson.update(), remove_entry(), check_up_to_date() and others call self.dump() potentially many times in loops / recursive installs). This can significantly slow down installs/updates and make repeated writes much more expensive. Consider keeping the default False and only running Prettier once at the end of a user-facing command, or refactoring so callers that do multiple updates can disable intermediate writes / formatting and do a single prettified dump at the end.
    def dump(self, run_prettier: bool = True) -> None:
        """
        Sort the modules.json, and write it to file
        """
        # Sort the modules.json
        if self.modules_json is None:
            self.load()
        if self.modules_json is not None:
            self.modules_json["repos"] = nf_core.utils.sort_dictionary(self.modules_json["repos"])
            if run_prettier:
                dump_json_with_prettier(self.modules_json_path, self.modules_json)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@mashehu mashehu left a comment

Choose a reason for hiding this comment

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

will make some commands slower, but can optimize for that in another PR.

@muffato muffato marked this pull request as draft February 27, 2026 11:28
@muffato
Copy link
Copy Markdown
Member Author

muffato commented Feb 27, 2026

Let me take a look and try to optimise it now.

@muffato muffato force-pushed the prettier_modules_json branch from 90d9e49 to fdeddee Compare March 2, 2026 09:19
@muffato
Copy link
Copy Markdown
Member Author

muffato commented Mar 2, 2026

New version. I'm adding run_prettier=True only to the functions that need it.
I tested installing, updating, and removing a module, patching and unpatching a module, installing, updating, and removing a sub-workflow.

@muffato muffato marked this pull request as ready for review March 2, 2026 09:28
@muffato muffato requested a review from mashehu March 2, 2026 09:28
@muffato
Copy link
Copy Markdown
Member Author

muffato commented Mar 2, 2026

Added support for one scenario: manual install of a module that was already installed via a sub-workflow.

@muffato muffato merged commit 8ae082d into dev Mar 2, 2026
112 checks passed
@muffato muffato deleted the prettier_modules_json branch March 2, 2026 12:05
@muffato
Copy link
Copy Markdown
Member Author

muffato commented Mar 2, 2026

Thank you !

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.

4 participants