Skip to content

Conversation

knguyen1
Copy link
Contributor

@knguyen1 knguyen1 commented Aug 16, 2025

Summary

Problem

The original CoverArtSetter implementation used dynamic method assignment in the constructor, causing type errors and violating DRY/SRP principles.

Solution

  • Refactored CoverArtSetter to use proper single dispatch pattern with @singledispatch
  • Moved implementation handlers to separate coverart_handlers.py module
  • Simplified main coverartsetter.py from 133 to 98 lines
  • Added comprehensive type hints throughout
  • Applied numpy-style docstrings with imperative mood
  • All tests pass (11/11) with improved coverage

Action

Additional actions required:

  • Update Picard documentation (please include a reference to this PR)
  • Other (please specify below)

@zas zas requested review from phw, rdswift and zas August 16, 2025 13:45
Copy link
Member

@phw phw left a comment

Choose a reason for hiding this comment

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

Great refactoring. LGTM

Copy link
Collaborator

@zas zas left a comment

Choose a reason for hiding this comment

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

LGTM

@zas zas merged commit 9a02f9b into metabrainz:master Aug 16, 2025
45 checks passed
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.

3 participants