Improve commandline metadata override

Change parse_metadata_from_string to yaml syntax
Add a special value to remove existing values when metadata is overlayed

.github/workflows/build.yaml

@@ -1,7 +1,6 @@
 name: CI
 
 env:
-  PKG_CONFIG_PATH: /usr/local/opt/icu4c/lib/pkgconfig:/opt/homebrew/opt/icu4c/lib/pkgconfig
   LC_COLLATE: en_US.UTF-8
 on:
   pull_request:
@@ -67,20 +66,19 @@ jobs:
       - name: Install macos dependencies
         run: |
           brew upgrade icu4c pkg-config || brew install icu4c pkg-config
-          # export PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig";
-          # export PATH="/usr/local/opt/icu4c/bin:/usr/local/opt/icu4c/sbin:$PATH"
         if: runner.os == 'macOS'
 
       - name: Install linux dependencies
         run: |
           sudo apt-get update && sudo apt-get upgrade && sudo apt-get install pkg-config libicu-dev libqt5gui5 libfuse2
-          # export PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig";
-          # export PATH="/usr/local/opt/icu4c/bin:/usr/local/opt/icu4c/sbin:$PATH"
         if: runner.os == 'Linux'
 
       - name: Build and install PyPi packages
         run: |
+          export PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig:/opt/homebrew/opt/icu4c/lib/pkgconfig${PKG_CONFIG_PATH+:$PKG_CONFIG_PATH}";
+          export PATH="/usr/local/opt/icu4c/bin:/usr/local/opt/icu4c/sbin${PATH+:$PATH}"
           python -m tox r -m build
+        shell: bash
 
       - name: Archive production artifacts
         uses: actions/upload-artifact@v3

.github/workflows/package.yaml

@@ -1,7 +1,6 @@
 name: Package
 
 env:
-  PKG_CONFIG_PATH: /usr/local/opt/icu4c/lib/pkgconfig:/opt/homebrew/opt/icu4c/lib/pkgconfig
   LC_COLLATE: en_US.UTF-8
 on:
   push:
@@ -34,21 +33,20 @@ jobs:
       - name: Install macos dependencies
         run: |
           brew upgrade && brew install icu4c pkg-config
-          # export PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig";
-          # export PATH="/usr/local/opt/icu4c/bin:/usr/local/opt/icu4c/sbin:$PATH"
         if: runner.os == 'macOS'
 
       - name: Install linux dependencies
         run: |
           sudo apt-get update && sudo apt-get upgrade && sudo apt-get install pkg-config libicu-dev libqt5gui5 libfuse2
-          # export PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig";
-          # export PATH="/usr/local/opt/icu4c/bin:/usr/local/opt/icu4c/sbin:$PATH"
         if: runner.os == 'Linux'
 
       - name: Build, Install and Test PyPi packages
         run: |
+          export PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig:/opt/homebrew/opt/icu4c/lib/pkgconfig${PKG_CONFIG_PATH+:$PKG_CONFIG_PATH}";
+          export PATH="/usr/local/opt/icu4c/bin:/usr/local/opt/icu4c/sbin${PATH+:$PATH}"
           python -m tox r
           python -m tox r -m release
+        shell: bash
         env:
           TWINE_USERNAME: __token__
           TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }}

.pre-commit-config.yaml

@@ -14,12 +14,12 @@ repos:
     hooks:
     -   id: setup-cfg-fmt
 -   repo: https://github.com/asottile/pyupgrade
-    rev: v3.15.1
+    rev: v3.15.2
     hooks:
     -   id: pyupgrade
         args: [--py39-plus]
 -   repo: https://github.com/PyCQA/autoflake
-    rev: v2.3.0
+    rev: v2.3.1
     hooks:
     -   id: autoflake
         args: [-i, --remove-all-unused-imports, --ignore-init-module-imports]
@@ -29,7 +29,7 @@ repos:
     -   id: isort
         args: [--af,--add-import, 'from __future__ import annotations']
 -   repo: https://github.com/psf/black
-    rev: 24.2.0
+    rev: 24.3.0
     hooks:
     -   id: black
 -   repo: https://github.com/PyCQA/flake8
@@ -38,7 +38,7 @@ repos:
     -   id: flake8
         additional_dependencies: [flake8-encodings, flake8-builtins, flake8-length, flake8-print, flake8-no-nested-comprehensions]
 -   repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.8.0
+    rev: v1.9.0
     hooks:
     -   id: mypy
         additional_dependencies: [types-setuptools, types-requests, settngs>=0.10.0]

comicapi/genericmetadata.py

@@ -25,17 +25,27 @@ import copy
 import dataclasses
 import logging
 from collections.abc import Sequence
-from typing import Any, TypedDict
+from enum import Enum, auto
+from typing import TYPE_CHECKING, Any, TypedDict, Union
 
 from typing_extensions import NamedTuple, Required
 
 from comicapi import utils
+from comicapi._url import Url, parse_url
 
-from ._url import Url, parse_url
+if TYPE_CHECKING:
+    Union
 
 logger = logging.getLogger(__name__)
 
 
+class __remove(Enum):
+    REMOVE = auto()
+
+
+REMOVE = __remove.REMOVE
+
+
 class PageType:
     """
     These page info classes are exactly the same as the CIX scheme, since
@@ -213,11 +223,15 @@ class GenericMetadata:
 
         def assign(cur: str, new: Any) -> None:
             if new is not None:
-                if isinstance(new, str) and len(new) == 0:
+                if new is REMOVE:
                     if isinstance(getattr(self, cur), (list, set)):
                         getattr(self, cur).clear()
                     else:
                         setattr(self, cur, None)
+                    return
+
+                if isinstance(new, str) and len(new) == 0:
+                    setattr(self, cur, None)
                 elif isinstance(new, (list, set)) and len(new) == 0:
                     pass
                 else:
@@ -281,8 +295,9 @@ class GenericMetadata:
         assign("_alternate_images", new_md._alternate_images)
 
     def overlay_credits(self, new_credits: list[Credit]) -> None:
-        if isinstance(new_credits, str) and len(new_credits) == 0:
+        if new_credits is REMOVE:
             self.credits = []
+            return
         for c in new_credits:
             primary = bool("primary" in c and c["primary"])
 
@@ -376,48 +391,45 @@ class GenericMetadata:
             elif val is not None:
                 vals.append((tag, val))
 
-        def add_attr_string(tag: str) -> None:
-            add_string(tag, getattr(self, tag))
-
-        add_attr_string("series")
-        add_attr_string("issue")
-        add_attr_string("issue_count")
-        add_attr_string("title")
-        add_attr_string("publisher")
-        add_attr_string("year")
-        add_attr_string("month")
-        add_attr_string("day")
-        add_attr_string("volume")
-        add_attr_string("volume_count")
+        add_string("series", self.series)
+        add_string("issue", self.issue)
+        add_string("issue_count", self.issue_count)
+        add_string("title", self.title)
+        add_string("publisher", self.publisher)
+        add_string("year", self.year)
+        add_string("month", self.month)
+        add_string("day", self.day)
+        add_string("volume", self.volume)
+        add_string("volume_count", self.volume_count)
         add_string("genres", ", ".join(self.genres))
-        add_attr_string("language")
-        add_attr_string("country")
-        add_attr_string("critical_rating")
-        add_attr_string("alternate_series")
-        add_attr_string("alternate_number")
-        add_attr_string("alternate_count")
-        add_attr_string("imprint")
-        add_attr_string("web_link")
-        add_attr_string("format")
-        add_attr_string("manga")
+        add_string("language", self.language)
+        add_string("country", self.country)
+        add_string("critical_rating", self.critical_rating)
+        add_string("alternate_series", self.alternate_series)
+        add_string("alternate_number", self.alternate_number)
+        add_string("alternate_count", self.alternate_count)
+        add_string("imprint", self.imprint)
+        add_string("web_links", [str(x) for x in self.web_links])
+        add_string("format", self.format)
+        add_string("manga", self.manga)
 
-        add_attr_string("price")
-        add_attr_string("is_version_of")
-        add_attr_string("rights")
-        add_attr_string("identifier")
-        add_attr_string("last_mark")
+        add_string("price", self.price)
+        add_string("is_version_of", self.is_version_of)
+        add_string("rights", self.rights)
+        add_string("identifier", self.identifier)
+        add_string("last_mark", self.last_mark)
 
         if self.black_and_white:
-            add_attr_string("black_and_white")
-        add_attr_string("maturity_rating")
-        add_attr_string("story_arcs")
-        add_attr_string("series_groups")
-        add_attr_string("scan_info")
+            add_string("black_and_white", self.black_and_white)
+        add_string("maturity_rating", self.maturity_rating)
+        add_string("story_arcs", self.story_arcs)
+        add_string("series_groups", self.series_groups)
+        add_string("scan_info", self.scan_info)
         add_string("characters", ", ".join(self.characters))
         add_string("teams", ", ".join(self.teams))
         add_string("locations", ", ".join(self.locations))
-        add_attr_string("description")
-        add_attr_string("notes")
+        add_string("description", self.description)
+        add_string("notes", self.notes)
 
         add_string("tags", ", ".join(self.tags))
 

comicapi/utils.py

@@ -33,7 +33,8 @@ from comicfn2dict import comicfn2dict
 import comicapi.data
 from comicapi import filenamelexer, filenameparser
 
-from ._url import Url, parse_url
+from ._url import Url as Url
+from ._url import parse_url as parse_url
 
 try:
     import icu

comictaggerlib/ctsettings/types.py

@@ -2,12 +2,95 @@ from __future__ import annotations
 
 import argparse
 import pathlib
+import sys
+import types
+import typing
+from collections.abc import Collection, Mapping
+from typing import Any
 
+import yaml
 from appdirs import AppDirs
 
 from comicapi import utils
 from comicapi.comicarchive import metadata_styles
-from comicapi.genericmetadata import GenericMetadata
+from comicapi.genericmetadata import REMOVE, GenericMetadata
+
+if sys.version_info < (3, 10):
+
+    @typing.no_type_check
+    def get_type_hints(obj, globalns=None, localns=None, include_extras=False):
+        if getattr(obj, "__no_type_check__", None):
+            return {}
+        # Classes require a special treatment.
+        if isinstance(obj, type):
+            hints = {}
+            for base in reversed(obj.__mro__):
+                if globalns is None:
+                    base_globals = getattr(sys.modules.get(base.__module__, None), "__dict__", {})
+                else:
+                    base_globals = globalns
+                ann = base.__dict__.get("__annotations__", {})
+                if isinstance(ann, types.GetSetDescriptorType):
+                    ann = {}
+                base_locals = dict(vars(base)) if localns is None else localns
+                if localns is None and globalns is None:
+                    # This is surprising, but required.  Before Python 3.10,
+                    # get_type_hints only evaluated the globalns of
+                    # a class.  To maintain backwards compatibility, we reverse
+                    # the globalns and localns order so that eval() looks into
+                    # *base_globals* first rather than *base_locals*.
+                    # This only affects ForwardRefs.
+                    base_globals, base_locals = base_locals, base_globals
+                for name, value in ann.items():
+                    if value is None:
+                        value = type(None)
+                    if isinstance(value, str):
+                        if "|" in value:
+                            value = "Union[" + value.replace(" |", ",") + "]"
+                        value = typing.ForwardRef(value, is_argument=False, is_class=True)
+                    value = typing._eval_type(value, base_globals, base_locals)
+                    hints[name] = value
+            return hints if include_extras else {k: typing._strip_annotations(t) for k, t in hints.items()}
+
+        if globalns is None:
+            if isinstance(obj, types.ModuleType):
+                globalns = obj.__dict__
+            else:
+                nsobj = obj
+                # Find globalns for the unwrapped object.
+                while hasattr(nsobj, "__wrapped__"):
+                    nsobj = nsobj.__wrapped__
+                globalns = getattr(nsobj, "__globals__", {})
+            if localns is None:
+                localns = globalns
+        elif localns is None:
+            localns = globalns
+        hints = getattr(obj, "__annotations__", None)
+        if hints is None:
+            # Return empty annotations for something that _could_ have them.
+            if isinstance(obj, typing._allowed_types):
+                return {}
+            else:
+                raise TypeError("{!r} is not a module, class, method, " "or function.".format(obj))
+        hints = dict(hints)
+        for name, value in hints.items():
+            if value is None:
+                value = type(None)
+            if isinstance(value, str):
+                if "|" in value:
+                    value = "Union[" + value.replace(" |", ",") + "]"
+                # class-level forward refs were handled above, this must be either
+                # a module-level annotation or a function argument annotation
+                value = typing.ForwardRef(
+                    value,
+                    is_argument=not isinstance(obj, types.ModuleType),
+                    is_class=False,
+                )
+            hints[name] = typing._eval_type(value, globalns, localns)
+        return hints if include_extras else {k: typing._strip_annotations(t) for k, t in hints.items()}
+
+else:
+    from typing import get_type_hints
 
 
 class ComicTaggerPaths(AppDirs):
@@ -84,50 +167,73 @@ def metadata_type(types: str) -> list[str]:
 
 
 def parse_metadata_from_string(mdstr: str) -> GenericMetadata:
-    """The metadata string is a comma separated list of name-value pairs
-    The names match the attributes of the internal metadata struct (for now)
-    The caret is the special "escape character", since it's not common in
-    natural language text
 
-    example = "series=Kickers^, Inc. ,issue=1, year=1986"
-    """
+    def get_type(key: str, tt: Any = get_type_hints(GenericMetadata)) -> Any:
+        t: Any = tt.get(key, None)
+        if t is None:
+            return None
+        if getattr(t, "__origin__", None) is typing.Union and len(t.__args__) == 2 and t.__args__[1] is type(None):
+            t = t.__args__[0]
+        elif isinstance(t, types.GenericAlias) and issubclass(t.mro()[0], Collection):
+            t = t.mro()[0], t.__args__[0]
 
-    escaped_comma = "^,"
-    escaped_equals = "^="
-    replacement_token = "<_~_>"
+        if isinstance(t, tuple) and issubclass(t[1], dict):
+            return (t[0], dict)
+        if isinstance(t, type) and issubclass(t, dict):
+            return dict
+        return t
+
+    def convert_value(t: type, value: Any) -> Any:
+        if not isinstance(value, t):
+            if isinstance(value, (Mapping)):
+                value = t(**value)
+            elif not isinstance(value, str) and isinstance(value, (Collection)):
+                value = t(*value)
+            else:
+                try:
+                    if t is utils.Url and isinstance(value, str):
+                        value = utils.parse_url(value)
+                    else:
+                        value = t(value)
+                except (ValueError, TypeError):
+                    raise argparse.ArgumentTypeError(f"Invalid syntax for tag '{key}'")
+        return value
 
     md = GenericMetadata()
 
-    # First, replace escaped commas with with a unique token (to be changed back later)
-    mdstr = mdstr.replace(escaped_comma, replacement_token)
-    tmp_list = utils.split(mdstr, ",")
-    md_list = []
-    for item in tmp_list:
-        item = item.replace(replacement_token, ",")
-        md_list.append(item)
+    if not mdstr:
+        return md
+    if mdstr[0] != "{":
+        mdstr = "{" + mdstr + "}"
 
-    # Now build a nice dict from the list
-    md_dict = {}
-    for item in md_list:
-        # Make sure to fix any escaped equal signs
-        i = item.replace(escaped_equals, replacement_token)
-        key, _, value = i.partition("=")
-        value = value.replace(replacement_token, "=").strip()
-        key = key.strip()
-        if key.casefold() == "credit":
-            cred_attribs = utils.split(value, ":")
-            role = cred_attribs[0]
-            person = cred_attribs[1] if len(cred_attribs) > 1 else ""
-            primary = len(cred_attribs) > 2
-            md.add_credit(person.strip(), role.strip(), primary)
-        else:
-            md_dict[key] = value
+    md_dict = yaml.safe_load(mdstr)
 
+    empty = True
     # Map the dict to the metadata object
     for key, value in md_dict.items():
-        if not hasattr(md, key):
-            raise argparse.ArgumentTypeError(f"'{key}' is not a valid tag name")
-        else:
-            md.is_empty = False
+        if hasattr(md, key):
+            t = get_type(key)
+            if value is None:
+                value = REMOVE
+            elif isinstance(t, tuple):
+                if value == "" or value is None:
+                    value = t[0]()
+                else:
+                    if isinstance(value, str):
+                        values: list[Any] = value.split("::")
+                    if not isinstance(value, Collection):
+                        raise argparse.ArgumentTypeError(f"Invalid syntax for tag '{key}'")
+                    values = list(value)
+                    for idx, v in enumerate(values):
+                        if not isinstance(v, t[1]):
+                            values[idx] = convert_value(t[1], v)
+                    value = t[0](values)
+            elif value is not None:
+                value = convert_value(t, value)
+
+            empty = False
             setattr(md, key, value)
+        else:
+            raise argparse.ArgumentTypeError(f"'{key}' is not a valid tag name")
+    md.is_empty = empty
     return md

comictaggerlib/gui.py

@@ -24,9 +24,14 @@ try:
         """
         if QtWidgets.QApplication.instance() is not None:
             errorbox = QtWidgets.QMessageBox()
+            errorbox.setStandardButtons(
+                QtWidgets.QMessageBox.StandardButton.Abort | QtWidgets.QMessageBox.StandardButton.Ignore
+            )
             errorbox.setText(log_msg)
-            errorbox.exec()
-            QtWidgets.QApplication.exit(1)
+            if errorbox.exec() == QtWidgets.QMessageBox.StandardButton.Abort:
+                QtWidgets.QApplication.exit(1)
+            else:
+                logger.warning("Exception ignored")
         else:
             logger.debug("No QApplication instance available.")
 

comictaggerlib/issueidentifier.py

@@ -643,7 +643,10 @@ class IssueIdentifier:
                     )
                     final_cover_matching.remove(match)
 
-        best_score = final_cover_matching[0].distance
+        if final_cover_matching:
+            best_score = final_cover_matching[0].distance
+        else:
+            best_score = 0
         if best_score >= self.min_score_thresh:
             if len(final_cover_matching) == 1:
                 self.log_msg("No matching pages in the issue.")

comictaggerlib/taggerwindow.py

@@ -848,7 +848,10 @@ class TaggerWindow(QtWidgets.QMainWindow):
     def metadata_to_form(self) -> None:
         def assign_text(field: QtWidgets.QLineEdit | QtWidgets.QTextEdit, value: Any) -> None:
             if value is not None:
-                field.setText(str(value))
+                if isinstance(field, QtWidgets.QTextEdit) and False:
+                    field.setPlainText(str(value))
+                else:
+                    field.setText(str(value))
 
         md = self.metadata
 

setup.cfg

@@ -45,6 +45,7 @@ install_requires =
     pathvalidate
     pillow>=9.1.0,<10
     pyrate-limiter>=2.6,<3
+    pyyaml
     rapidfuzz>=2.12.0
     requests==2.*
     settngs==0.10.0
@@ -317,6 +318,7 @@ disallow_incomplete_defs = true
 disallow_untyped_defs = true
 warn_redundant_casts = true
 warn_unused_ignores = true
+disable_error_code = import-untyped
 
 [mypy-testing.*]
 disallow_untyped_defs = false

tests/ctsettings_test.py

@@ -7,13 +7,13 @@ import comictaggerlib.ctsettings.types
 
 md_strings = (
     ("", comicapi.genericmetadata.md_test.replace()),
-    ("year=", comicapi.genericmetadata.md_test.replace(year=None)),
-    ("year=2009", comicapi.genericmetadata.md_test.replace(year="2009")),
-    ("series=", comicapi.genericmetadata.md_test.replace(series=None)),
-    ("series_aliases=", comicapi.genericmetadata.md_test.replace(series_aliases=set())),
-    ("black_and_white=", comicapi.genericmetadata.md_test.replace(black_and_white=None)),
-    ("credits=", comicapi.genericmetadata.md_test.replace(credits=[])),
-    ("story_arcs=", comicapi.genericmetadata.md_test.replace(story_arcs=[])),
+    ("year:", comicapi.genericmetadata.md_test.replace(year=None)),
+    ("year: 2009", comicapi.genericmetadata.md_test.replace(year=2009)),
+    ("series:", comicapi.genericmetadata.md_test.replace(series=None)),
+    ("series_aliases:", comicapi.genericmetadata.md_test.replace(series_aliases=set())),
+    ("black_and_white:", comicapi.genericmetadata.md_test.replace(black_and_white=None)),
+    ("credits:", comicapi.genericmetadata.md_test.replace(credits=[])),
+    ("story_arcs:", comicapi.genericmetadata.md_test.replace(story_arcs=[])),
 )
 
 

tests/genericmetadata_test.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import textwrap
+
 import pytest
 
 import comicapi.genericmetadata
@@ -40,3 +42,46 @@ def test_add_credit_primary():
 @pytest.mark.parametrize("md, role, expected", credits)
 def test_get_primary_credit(md, role, expected):
     assert md.get_primary_credit(role) == expected
+
+
+def test_str(md):
+    expected = textwrap.dedent(
+        """\
+        series:           Cory Doctorow's Futuristic Tales of the Here and Now
+        issue:            1
+        issue_count:      6
+        title:            Anda's Game
+        publisher:        IDW Publishing
+        year:             2007
+        month:            10
+        day:              1
+        volume:           1
+        genres:           Sci-Fi
+        language:         en
+        critical_rating:  3.0
+        alternate_series: Tales
+        alternate_number: 2
+        alternate_count:  7
+        imprint:          craphound.com
+        web_links:        ['https://comicvine.gamespot.com/cory-doctorows-futuristic-tales-of-the-here-and-no/4000-140529/']
+        format:           Series
+        manga:            No
+        maturity_rating:  Everyone 10+
+        story_arcs:       ['Here and Now']
+        series_groups:    ['Futuristic Tales']
+        scan_info:        (CC BY-NC-SA 3.0)
+        characters:       Anda
+        teams:            Fahrenheit
+        locations:        lonely  cottage
+        description:      For 12-year-old Anda, getting paid real money to kill the characters of players who were cheating in her favorite online computer game was a win-win situation. Until she found out who was paying her, and what those characters meant to the livelihood of children around the world.
+        notes:            Tagged with ComicTagger 1.3.2a5 using info from Comic Vine on 2022-04-16 15:52:26. [Issue ID 140529]
+        credit:           Writer: Dara Naraghi
+        credit:           Penciller: Esteve Polls
+        credit:           Inker: Esteve Polls
+        credit:           Letterer: Neil Uyetake
+        credit:           Cover: Sam Kieth
+        credit:           Editor: Ted Adams
+    """
+    )
+
+    assert str(md) == expected