beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.57k stars 1.8k forks source link

Fix `_legalize_path` types #5224

Open snejus opened 1 month ago

snejus commented 1 month ago

Description

Part 2 of the work fixing types in beets.util.__init__ #5215.

Mypy was not happy here because _legalize_stage function implementation concatenates path and extension parameters, implying that their types need to match.

You can see that initially path parameter was defined as a str while extension was bytes.

In reality, depending on the fragment parameter value, extension was sometimes provided as a str and sometimes as bytes. The same parameter decided whether path gets converted into bytes within _legalize_stage implementation. No surprise that mypy was confused here.

_legalize_stage is only used within Item.destination method implementation which is where fragment is defined. I determined that the fragment parameter controls the form of the output path:

Given the above, the change

  1. Renames fragment parameter to relative_to_libdir for clarity

  2. Makes Item.destination to return the same type in both cases. I picked bytes since that's the type that majority of the code using this method expects.

    I converted the output path to str for the code that has been expecting a string there.

  3. Decouples _legalize_stage and _legalize_path implementations from the relative_to_libdir. The logic now uses str type only.

github-actions[bot] commented 1 month ago

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.