search

beetbox / beets

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

Add fetchart typing #5213

Open wisp3rwind opened 2 months ago

wisp3rwind commented 2 months ago

Add typings to fetchart. there are a few rough edges requiring cast and enums could be better represented, but this is a start.

wisp3rwind commented 1 month ago

Ok, I think I addressed everything. I went quite far in trying to express things via types, at least for this being Python.

There's one more thing that this reminded me of (but which is out of scope in this PR): At some point, the ImageAction should really be turned into a bitfield/flags rather than an enum (also, this enum might even move into the artresizer module), cf. https://github.com/beetbox/beets/pull/4133#issuecomment-968268133

wisp3rwind commented 1 month ago

I've just seen https://github.com/beetbox/beets/pull/5152, which I've completely missed up to now, and which is badly conflicting with what I'm doing here. However, while it's great that there's some momentum on the issue that #5152 tackles, I'm not convinced of the approach.

In contrast, what I'm doing here would fit nicely with eventually pushing the multiple-action image conversion completely to the artresizer.

Dr-Blank commented 1 month ago

I've just seen #5152, which I've completely missed up to now, and which is badly conflicting with what I'm doing here. However, while it's great that there's some momentum on the issue that #5152 tackles, I'm not convinced of the approach.

In contrast, what I'm doing here would fit nicely with eventually pushing the multiple-action image conversion completely to the artresizer.

The only issue with pushing multiple flags is that image operations are not mutually exclusive. Reformating affects size.

For e.g., the size of the PNG image changes drastically when converted to JPEG. So if my configuration is to always use JPEG images of max size 5 MB, the flags for a 15 MB PNG would be [REFORMAT, RESIZE] but after reformat if the size would drop to below 5 MB, the RESIZE would be unnecessary, hence a evaluation is needed after every step.