Skip to content

Commit

Permalink
implement fix for nested FilterComparers + other fixes (#37)
Browse files Browse the repository at this point in the history
* implement fix for nested FilterComparers + other fixes

* update release history

* playlist path finder in library uses get_filepaths + fix process method output on FilterComparers
  • Loading branch information
geo-martino authored Mar 12, 2024
1 parent 25363cd commit 547fcee
Show file tree
Hide file tree
Showing 38 changed files with 429 additions and 200 deletions.
1 change: 1 addition & 0 deletions .idea/inspectionProfiles/Project_Default.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions docs/release-history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ and this project adheres to `Semantic Versioning <https://semver.org/spec/v2.0.0
0.8.1
=====

Changed
-------

* :py:class:`.ItemSorter` now accepts ``shuffle_weight`` between -1 and 1 instead of 0 and 1.
This parameter's logic has not yet been implemented so no changes to functionality have been made yet.
* Move :py:meth:`.get_filepaths` from :py:class:`.LocalTrack` to super class :py:class:`.File`

Documentation
-------------

Expand All @@ -48,6 +55,13 @@ Fixed
* :py:func:`.align_string` function now handles combining unicode characters properly for fixed-width fonts
* :py:meth:`.LocalTrack.get_filepaths` on LocalTrack no longer returns paths from ``$RECYCLE.BIN`` folders.
These are deleted files and were causing the package to crash when trying to load them.
* :py:meth:`.PrettyPrinter.json` and :py:meth:`.PrettyPrinter._to_str` converts attribute keys to string
to ensure safe json/str/repr output
* :py:class:`.FilterMatcher` and :py:class:`.FilterComparers` now correctly import conditions from XML playlist files.
Previously, these filters could not import nested match conditions from files.
Changes to logic also made to :py:meth:`.Comparer.from_xml` to accommodate.
* :py:class:`.XMLLibraryParser` now handles empty arrays correctly. Previously would crash.
* Fixed :py:class:`.Comparer` dynamic process method alternate names for ``in_the_last`` and ``not_in_the_last``

Removed
-------
Expand Down
3 changes: 1 addition & 2 deletions musify/local/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@
from __future__ import annotations

import logging
import sys
from abc import ABCMeta, abstractmethod
from collections.abc import Mapping, Collection, Iterable, Container
from datetime import datetime
from glob import glob
from os.path import splitext, join, basename, exists, isdir
from typing import Any

import sys

from musify.local.base import LocalItem
from musify.local.exception import LocalCollectionError
from musify.local.track import LocalTrack, SyncResultTrack, load_track, TRACK_FILETYPES
Expand Down
14 changes: 13 additions & 1 deletion musify/local/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
from abc import ABCMeta, abstractmethod
from collections.abc import Hashable, Collection, Iterable
from datetime import datetime
from glob import glob
from http.client import HTTPResponse
from io import BytesIO
from os import sep
from os.path import splitext, basename, dirname, getsize, getmtime, getctime, exists
from os.path import splitext, basename, dirname, getsize, getmtime, getctime, exists, join
from pathlib import Path
from typing import Any
from urllib.error import URLError
Expand Down Expand Up @@ -102,6 +103,17 @@ def _validate_type(self, path: str) -> None:
f"Use only: {', '.join(self.valid_extensions)}"
)

@classmethod
def get_filepaths(cls, folder: str) -> set[str]:
"""Get all files in a given folder that match this File object's valid filetypes recursively."""
paths = set()

for ext in cls.valid_extensions:
paths |= set(glob(join(folder, "**", f"*{ext}"), recursive=True, include_hidden=True))

# do not return paths in the recycle bin in Windows-based folders
return {path for path in paths if "$RECYCLE.BIN" not in path}

@abstractmethod
def load(self, *args, **kwargs) -> Any:
"""Load the file to this object"""
Expand Down
15 changes: 5 additions & 10 deletions musify/local/library/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@
import itertools
from collections.abc import Collection, Mapping, Iterable
from functools import reduce
from glob import glob
from os.path import splitext, join, exists, basename, dirname
from typing import Any

from musify.local.collection import LocalCollection, LocalFolder, LocalAlbum, LocalArtist, LocalGenres
from musify.local.file import PathMapper, PathStemMapper
from musify.local.playlist import PLAYLIST_FILETYPES, LocalPlaylist, load_playlist
from musify.local.playlist import LocalPlaylist, load_playlist, PLAYLIST_CLASSES
from musify.local.track import TRACK_CLASSES, LocalTrack, load_track
from musify.local.track.field import LocalTrackField
from musify.processors.base import Filter
Expand Down Expand Up @@ -135,14 +134,10 @@ def playlist_folder(self, value: str | None):
self._playlist_folder: str = value.rstrip("\\/")
self._playlist_paths = None

playlists = {}
for filetype in PLAYLIST_FILETYPES:
paths = glob(join(self._playlist_folder, "**", f"*{filetype}"), recursive=True)
entry = {
splitext(basename(path.removeprefix(self._playlist_folder)))[0]: path
for path in paths
}
playlists |= entry
playlists = {
splitext(basename(path.removeprefix(self._playlist_folder)))[0]: path
for cls in PLAYLIST_CLASSES for path in cls.get_filepaths(self._playlist_folder)
}

pl_total = len(playlists)
pl_filtered = self.playlist_filter(playlists)
Expand Down
11 changes: 7 additions & 4 deletions musify/local/library/musicbee.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import hashlib
import re
import urllib.parse
from collections.abc import Iterable, Mapping, Sequence, Generator, Collection
from collections.abc import Iterable, Mapping, Sequence, Collection, Iterator
from datetime import datetime
from os.path import join, exists, normpath
from typing import Any
Expand Down Expand Up @@ -172,7 +172,7 @@ def save(self, dry_run: bool = True, *_, **__) -> dict[str, Any]:

tracks: dict[int, dict[str, Any]] = {}
max_track_id = max(id_ for id_, _ in track_id_map.values()) if track_id_map else 0
for i, track in enumerate(self.tracks, max(1, max_track_id)):
for i, track in enumerate(self.tracks, max(1, max_track_id + 1)):
track_id, persistent_id = track_id_map.get(track, [i, None])
tracks[track_id] = self.track_to_xml(track, track_id=track_id, persistent_id=persistent_id)
track_id_map[track] = (tracks[track_id]["Track ID"], tracks[track_id]["Persistent ID"])
Expand All @@ -184,7 +184,7 @@ def save(self, dry_run: bool = True, *_, **__) -> dict[str, Any]:

playlists: list[dict[str, Any]] = []
max_playlist_id = max(id_ for id_, _ in playlist_id_map.values()) if playlist_id_map else 0
for i, (name, playlist) in enumerate(self.playlists.items(), max_playlist_id):
for i, (name, playlist) in enumerate(self.playlists.items(), max_playlist_id + 1):
playlist_id, persistent_id = playlist_id_map.get(name, [i, None])
playlist = self.playlist_to_xml(
playlist, tracks=track_id_map, playlist_id=playlist_id, persistent_id=persistent_id
Expand Down Expand Up @@ -370,7 +370,7 @@ def from_xml_path(path: str) -> str:
"""Clean the file paths as found in the MusicBee XML library file to a standard system path"""
return normpath(urllib.parse.unquote(path.removeprefix("file://localhost/")))

def _iter_elements(self) -> Generator[etree.Element, [], []]:
def _iter_elements(self) -> Iterator[etree.Element]:
for event, element in self._iterparse:
yield element

Expand Down Expand Up @@ -412,6 +412,9 @@ def _parse_element(self, element: etree._Element | None = None) -> Any:
def _parse_array(self, element: etree._Element | None = None) -> list[Any]:
array = []

if element is not None and element.tag == "array" and element.text is None:
return array # array is empty, skip processing

for elem in self._iter_elements():
if elem is None or elem.tag == "array":
break
Expand Down
15 changes: 0 additions & 15 deletions musify/local/track/base/track.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import os
from abc import ABCMeta
from copy import deepcopy
from glob import glob
from os.path import join, exists, dirname
from typing import Any, Self

Expand Down Expand Up @@ -40,20 +39,6 @@ class LocalTrack[T: mutagen.FileType](TagWriter, metaclass=ABCMeta):
def file(self):
return self._file

@classmethod
def get_filepaths(cls, library_folder: str) -> set[str]:
"""Get all files in a given library that match this Track object's valid filetypes."""
paths = set()

for ext in cls.valid_extensions:
# first glob doesn't get filenames that start with a period
paths |= set(glob(join(library_folder, "**", f"*{ext}"), recursive=True))
# second glob only picks up filenames that start with a period
paths |= set(glob(join(library_folder, "*", "**", f".*{ext}"), recursive=True))

# do not return paths in the recycle bin in Windows-based folders
return {path for path in paths if "$RECYCLE.BIN" not in path}

def __init__(self, file: str | T, remote_wrangler: RemoteDataWrangler = None):
super().__init__(remote_wrangler=remote_wrangler)

Expand Down
33 changes: 16 additions & 17 deletions musify/processors/compare.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,24 +88,23 @@ def expected(self, value: Sequence[Any] | None):
self._expected = to_collection(value, list)

@classmethod
def from_xml(cls, xml: Mapping[str, Any], **__) -> list[Self]:
conditions = xml["SmartPlaylist"]["Source"]["Conditions"]["Condition"]
conditions: tuple[Mapping[str, str], ...] = to_collection(conditions)

objs = []
for condition in conditions:
field_str = condition.get("@Field", "None")
field: Field = field_name_map.get(field_str)
if field is None:
raise FieldError("Unrecognised field name", field=field_str)
def from_xml(cls, xml: Mapping[str, Any], **__) -> Self:
"""
Initialise object from XML playlist data.
expected: tuple[str, ...] | None = tuple(v for k, v in condition.items() if k.startswith("@Value"))
if len(expected) == 0 or expected[0] == "[playing track]":
expected = None
:param xml: The loaded XML object for this playlist.
This function expects to be given only the XML part related to one Comparer condition.
"""
field_str = xml.get("@Field", "None")
field: Field = field_name_map.get(field_str)
if field is None:
raise FieldError("Unrecognised field name", field=field_str)

objs.append(cls(condition=condition["@Comparison"], expected=expected, field=field))
expected: tuple[str, ...] | None = tuple(v for k, v in xml.items() if k.startswith("@Value"))
if len(expected) == 0 or expected[0] == "[playing track]":
expected = None

return objs
return cls(condition=xml["@Comparison"], expected=expected, field=field)

def to_xml(self, **kwargs) -> Mapping[str, Any]:
raise NotImplementedError
Expand Down Expand Up @@ -252,11 +251,11 @@ def _is(self, value: Any | None, expected: Sequence[Any] | None) -> bool:
def _is_not(self, value: Any | None, expected: Sequence[Any] | None) -> bool:
return not self._is(value=value, expected=expected)

@dynamicprocessormethod("greater_than", "_is_in_the_last")
@dynamicprocessormethod("greater_than", "in_the_last")
def _is_after(self, value: Any | None, expected: Sequence[Any] | None) -> bool:
return value > expected[0] if value is not None and expected[0] is not None else False

@dynamicprocessormethod("less_than", "_is_not_in_the_last")
@dynamicprocessormethod("less_than", "not_in_the_last")
def _is_before(self, value: Any | None, expected: Sequence[Any] | None) -> bool:
return value < expected[0] if value is not None and expected[0] is not None else False

Expand Down
41 changes: 33 additions & 8 deletions musify/processors/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

from __future__ import annotations

from collections.abc import Collection, Sequence
from typing import Any
from collections.abc import Collection, Sequence, Mapping
from typing import Any, Self

from musify.processors.base import Filter, FilterComposite
from musify.processors.compare import Comparer
Expand Down Expand Up @@ -59,10 +59,21 @@ class FilterComparers[T: str | Nameable](Filter[T]):
def ready(self):
return len(self.comparers) > 0

def __init__(self, comparers: Collection[Comparer] = (), match_all: bool = True, *_, **__):
def __init__(
self,
comparers: Collection[Comparer] | Mapping[Comparer, tuple[bool, Self]] = (),
match_all: bool = True,
*_,
**__
):
super().__init__()
if not isinstance(comparers, Mapping):
comparers = {comparer: (False, FilterComparers()) for comparer in comparers}

#: The comparers to use when processing for this filter
self.comparers: Collection[Comparer] = comparers
#: When a mapping with other :py:class:`FilterComparers` is given,
#: will apply this sub-filter to all parent filters when processing
self.comparers: Mapping[Comparer, tuple[bool, Self]] = comparers
#: When true, only include those items that match on all comparers
self.match_all: bool = match_all

Expand All @@ -77,14 +88,28 @@ def run_comparer(c: Comparer, v: T) -> bool:
"""Run the comparer ``c`` for the given value ``v``"""
return c(self.transform(v), reference=reference) if c.expected is None else c(self.transform(v))

def run_sub_filter(s: FilterComparers, c: bool, v: list[T]) -> Collection[T]:
"""Run the sub_filter ``s`` for the given value ``v`` and return the results"""
if not s.ready:
return v
elif c:
return sub_filter(v, reference=reference)
else:
v.extend([value for value in sub_filter(values, reference=reference) if value not in v])
return v

if self.match_all:
for comparer in self.comparers:
values = [value for value in values if run_comparer(comparer, value)]
for comparer, (combine, sub_filter) in self.comparers.items():
matched = [value for value in values if run_comparer(comparer, value)]
values = run_sub_filter(sub_filter, combine, matched)
return values

matches = []
for comparer in self.comparers:
matches.extend(value for value in values if run_comparer(comparer, value) and value not in matches)
for comparer, (combine, sub_filter) in self.comparers.items():
matched = [value for value in values if value not in matches and run_comparer(comparer, value)]
matched = run_sub_filter(sub_filter, combine, matched)
matches.extend([value for value in matched if value not in matches])

return matches

def as_dict(self) -> dict[str, Any]:
Expand Down
35 changes: 25 additions & 10 deletions musify/processors/filter_matcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from __future__ import annotations

import logging
from collections.abc import Collection, Mapping, Sequence, Callable
from collections.abc import Collection, Mapping, Callable
from dataclasses import field, dataclass
from typing import Any

Expand All @@ -18,6 +18,7 @@
from musify.shared.core.enum import Fields
from musify.shared.core.misc import Result
from musify.shared.logger import MusifyLogger
from musify.shared.utils import to_collection


@dataclass(frozen=True)
Expand Down Expand Up @@ -52,7 +53,7 @@ class FilterMatcher[T: Any, U: Filter, V: Filter, X: FilterComparers](MusicBeePr
@classmethod
def from_xml(
cls, xml: Mapping[str, Any], path_mapper: PathMapper = PathMapper(), **__
) -> FilterMatcher[T, FilterDefinedList[T], FilterDefinedList[T], FilterComparers[T]]:
) -> FilterMatcher[str, FilterDefinedList[str], FilterDefinedList[str], FilterComparers[str]]:
"""
Initialise object from XML playlist.
Expand All @@ -69,18 +70,32 @@ def from_xml(
exclude_str: str = source.get("Exceptions") or ""
exclude = path_mapper.map_many(set(exclude_str.split("|")), check_existence=True)

comparers: Sequence[Comparer] = Comparer.from_xml(xml=xml)

if len(comparers) == 1:
# when user has not set an explicit comparer, there will still be an 'allow all' comparer
# check for this 'allow all' comparer and remove if present to speed up comparisons
c = comparers[0]
conditions = xml["SmartPlaylist"]["Source"]["Conditions"]
comparers: dict[Comparer, tuple[bool, FilterComparers]] = {}
for condition in to_collection(conditions["Condition"]):
if any(key in condition for key in {"And", "Or"}):
sub_combine = "And" in condition
sub_conditions = condition["And" if sub_combine else "Or"]
sub_comparers = [Comparer.from_xml(sub) for sub in to_collection(sub_conditions["Condition"])]
sub_filter = FilterComparers(
comparers=sub_comparers, match_all=sub_conditions["@CombineMethod"] == "All"
)
else:
sub_combine = False
sub_filter = FilterComparers()

comparers[Comparer.from_xml(xml=condition)] = (sub_combine, sub_filter)

if len(comparers) == 1 and not next(iter(comparers.values()))[1].ready:
# when user has not set an explicit comparer, a single empty 'allow all' comparer is assigned
# check for this 'allow all' comparer and remove it if present to speed up comparisons
c = next(iter(comparers))
if "contains" in c.condition.casefold() and len(c.expected) == 1 and not c.expected[0]:
comparers = ()
comparers = {}

filter_include = FilterDefinedList(values=[path.casefold() for path in include])
filter_exclude = FilterDefinedList(values=[path.casefold() for path in exclude])
filter_compare = FilterComparers(comparers, match_all=source["Conditions"]["@CombineMethod"] == "All")
filter_compare = FilterComparers(comparers, match_all=conditions["@CombineMethod"] == "All")

filter_include.transform = lambda x: path_mapper.map(x, check_existence=False).casefold()
filter_exclude.transform = lambda x: path_mapper.map(x, check_existence=False).casefold()
Expand Down
4 changes: 2 additions & 2 deletions musify/processors/sort.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class ItemSorter(MusicBeeProcessor):
* Map of ``{<tag/property>: <reversed>}``. If reversed is true, sort the ``tag/property`` in reverse.
:param shuffle_mode: The mode to use for shuffling.
:param shuffle_by: The field to shuffle by when shuffling.
:param shuffle_weight: The weights (between 0 and 1) to apply to shuffling modes that can use it.
:param shuffle_weight: The weights (between -1 and 1) to apply to shuffling modes that can use it.
This value will automatically be limited to within the accepted range 0 and 1.
"""

Expand Down Expand Up @@ -181,7 +181,7 @@ def __init__(
self.shuffle_mode: ShuffleMode | None
self.shuffle_mode = shuffle_mode if shuffle_mode in [ShuffleMode.NONE, ShuffleMode.RANDOM] else ShuffleMode.NONE
self.shuffle_by: ShuffleBy | None = shuffle_by
self.shuffle_weight = limit_value(shuffle_weight, floor=0, ceil=1)
self.shuffle_weight = limit_value(shuffle_weight, floor=-1, ceil=1)

def __call__(self, items: MutableSequence[Item]) -> None:
return self.sort(items=items)
Expand Down
Loading

0 comments on commit 547fcee

Please sign in to comment.