Skip to content

refactor: update plugin to follow Python best practices#9

Closed
ewels wants to merge 9 commits intomainfrom
claude/refactor-plugin-best-practices-xOSUS
Closed

refactor: update plugin to follow Python best practices#9
ewels wants to merge 9 commits intomainfrom
claude/refactor-plugin-best-practices-xOSUS

Conversation

@ewels
Copy link
Copy Markdown
Member

@ewels ewels commented Dec 16, 2025

Summary

Modernizes the MultiQC_SAV plugin to follow current Python best practices, aligned with the xenium-extra MultiQC plugin and MultiQC module guidelines.

Changes

Project Configuration

  • Modernize pyproject.toml with full project metadata, dependencies, and entry points
  • Remove setup.py (no longer needed with modern packaging)
  • Bump Python requirement to >=3.9
  • Add ruff and mypy configuration

Pre-commit & Linting

  • Add .pre-commit-config.yaml with ruff, prettier, and standard hooks
  • Add .prettierrc.js and .prettierignore for consistent formatting
  • Exclude test_data/ and docs/example/ from file-modifying hooks

GitHub Actions

  • Add lint.yaml workflow using pre-commit action
  • Add publish.yaml workflow for PyPI releases (trusted publishing)
  • Update linux.yaml with modern action versions and Python 3.9-3.12 matrix

Code Refactoring

  • Add comprehensive type hints throughout
  • Use f-strings consistently
  • Replace deprecated pkg_resources with importlib.metadata
  • Remove OrderedDict (use dict in Python 3.7+)
  • Use contextlib.suppress instead of try-except-pass
  • Wrap long description strings to satisfy line length limit

Documentation

  • Add CLAUDE.md for AI assistant guidance

Test plan

  • Pre-commit passes: pre-commit run --all-files
  • Ruff checks pass: ruff check . && ruff format --check .
  • Module runs on test data: pip install -e . && multiqc -m SAV test_data/MiSeq

- Modernize pyproject.toml with full project configuration
  - Add build-system specification
  - Configure ruff for linting and formatting
  - Add mypy configuration
  - Bump Python requirement to >=3.9

- Add pre-commit configuration
  - Include ruff-format and ruff-lint hooks
  - Add prettier for markdown/yaml formatting
  - Include standard hooks (trailing whitespace, merge conflict, etc.)

- Add GitHub Actions linting workflow
  - Separate lint.yaml for ruff and prettier checks
  - Update linux.yaml with modern action versions and Python 3.9-3.12

- Refactor Python code
  - Add comprehensive type hints throughout
  - Use f-strings consistently
  - Replace deprecated pkg_resources with importlib.metadata
  - Remove OrderedDict (use dict in Python 3.7+)
  - Use contextlib.suppress instead of try-except-pass
  - Improve docstrings and code organization

- Add CLAUDE.md for AI assistant guidance
- Simplify setup.py to minimal pyproject.toml wrapper
Replace separate ruff and prettier jobs with the official pre-commit
action. This ensures CI matches local pre-commit results exactly.
Remove redundant options that match defaults:
- target-version (inferred from requires-python)
- format options (all defaults)
- isort section (not needed)
- excessive lint rules

Keep only essential settings:
- line-length = 120
- select core rules (E, F, W, I, UP, SIM)
- ignore E501 (handled by formatter)
Remove E501 ignore rule - fix the actual long lines instead of ignoring them.
- Fix end-of-file newlines in XML files
- Fix trailing whitespace in HTML reports
- Normalize line endings (CRLF to LF)
- Apply prettier formatting to CLAUDE.md
- Remove setup.py (pyproject.toml handles everything)
- Add publish.yaml workflow triggered on GitHub releases
- Uses trusted publishing with OIDC (no API tokens needed)
Revert changes to test data XML files and add exclude patterns
to prevent pre-commit hooks from modifying test data and example files.
Revert docs/example HTML files and remove html from prettier types.
- Use --strict mode to catch issues
- Verify HTML report is generated for each test
- Check that SAV module appears in the log file
- Output to separate directories per sequencer type
@ewels ewels added the WIP Work in progress label Dec 17, 2025
@ewels
Copy link
Copy Markdown
Member Author

ewels commented Dec 17, 2025

@matthdsm - I made a start on modernising the codebase of this plugin a little. However, the CI jobs show that it's not finding the test data.

Any help would be much appreciated! Was it definitely working on master? Not sure if I broke it here or if it was already broken.

@matthdsm
Copy link
Copy Markdown
Collaborator

Hi @ewels,

I've also already done some work (and added new test data) in the dev branch.
I'm pretty sure the plugin worked on the available test data, however, I think I only ran manual tests 🤔 So not sure if the CI should work.

It's been quite a while, so I don't remember everything very well. As I said in #6 and mentioned in #5, picking up the input files is quite flaky...

@ewels
Copy link
Copy Markdown
Member Author

ewels commented Dec 18, 2025

That's the second time in a few weeks that I've done that, not checking to see if there's a dev branch on a repo 🤦🏻

I'll see if there's anything to salvage on this PR, but will probably just close it out.

@matthdsm
Copy link
Copy Markdown
Collaborator

No worries 😅 chances are your PR has more usable stuff than the dev branch

@ewels
Copy link
Copy Markdown
Member Author

ewels commented Dec 18, 2025

Closing in favour of #10

@ewels ewels closed this Dec 18, 2025
@ewels ewels deleted the claude/refactor-plugin-best-practices-xOSUS branch December 18, 2025 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WIP Work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants