From b0f598c72aaf3734981e0708051b4ad9ebfa3965 Mon Sep 17 00:00:00 2001 From: Balduin Landolt <33053745+BalduinLandolt@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:07:12 +0100 Subject: [PATCH] chore: resolve Ruff warnings A003, B023, D103 and PLR0911 (#684) Co-authored-by: Johannes Nussbaum <39048939+jnussbaum@users.noreply.github.com> --- .github/workflows/tests-on-push.yml | 2 +- .pre-commit-config.yaml | 4 - pyproject.toml | 27 +-- src/dsp_tools/cli/call_action.py | 2 +- src/dsp_tools/commands/excel2json/project.py | 10 +- .../commands/excel2xml/excel2xml_lib.py | 184 +++++++++--------- .../commands/project/models/listnode.py | 21 -- .../commands/project/models/ontology.py | 2 +- .../commands/project/models/propertyclass.py | 6 +- .../xmlupload/models/xmlpermission.py | 9 +- .../commands/xmlupload/models/xmlresource.py | 6 +- .../xmlupload/resource_create_client.py | 6 +- .../commands/xmlupload/resource_multimedia.py | 2 +- .../stash/stash_circular_references.py | 6 +- src/dsp_tools/commands/xmlupload/xmlupload.py | 20 +- src/dsp_tools/models/helpers.py | 2 +- src/dsp_tools/models/langstring.py | 66 +++---- .../commands/excel2json/test_lists.py | 6 - .../commands/excel2json/test_project.py | 2 - .../commands/excel2xml/test_excel2xml_lib.py | 150 +++++++++----- .../xmlupload/test_list_client_live.py | 4 - 21 files changed, 283 insertions(+), 254 deletions(-) diff --git a/.github/workflows/tests-on-push.yml b/.github/workflows/tests-on-push.yml index d16edc890..4013f14ef 100644 --- a/.github/workflows/tests-on-push.yml +++ b/.github/workflows/tests-on-push.yml @@ -67,7 +67,7 @@ jobs: run: > poetry run ruff check . --output-format=github - --ignore=A002,A003,B023,D101,D102,D103,PLR0911,PLR0912,PLR0913,PLR0915,PLR2004,PLR5501,PLW0603 + --ignore=A002,D101,D102,PLR0912,PLR0913,PLR0915,PLR2004,PLR5501,PLW0603 - name: Formatting with ruff run: poetry run ruff format . - name: Linting with mypy diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b7bd81e9c..6bd7aee46 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -8,12 +8,8 @@ repos: - id: ruff args: [ --ignore=A002, - --ignore=A003, - --ignore=B023, --ignore=D101, --ignore=D102, - --ignore=D103, - --ignore=PLR0911, --ignore=PLR0912, --ignore=PLR0913, --ignore=PLR0915, diff --git a/pyproject.toml b/pyproject.toml index 7eb15e63e..4c1302391 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -104,20 +104,20 @@ pythonpath = [".", "src", "test"] [tool.mypy] -ignore_missing_imports = true # TODO: deactivate this +ignore_missing_imports = true # TODO: deactivate this show_column_numbers = true strict = true exclude = [ - "src/dsp_tools/models/helpers.py", # TODO: activate this - "src/dsp_tools/models/langstring.py", # TODO: activate this - "src/dsp_tools/commands/project/models/group.py", # TODO: activate this - "src/dsp_tools/commands/project/models/listnode.py", # TODO: activate this - "src/dsp_tools/commands/project/models/ontology.py", # TODO: activate this - "src/dsp_tools/commands/project/models/project.py", # TODO: activate this - "src/dsp_tools/models/projectContext.py", # TODO: activate this - "src/dsp_tools/commands/project/models/propertyclass.py", # TODO: activate this - "src/dsp_tools/commands/project/models/resourceclass.py", # TODO: activate this - "src/dsp_tools/commands/project/models/user.py", # TODO: activate this + "src/dsp_tools/models/helpers.py", # TODO: activate this + "src/dsp_tools/models/langstring.py", # TODO: activate this + "src/dsp_tools/commands/project/models/group.py", # TODO: activate this + "src/dsp_tools/commands/project/models/listnode.py", # TODO: activate this + "src/dsp_tools/commands/project/models/ontology.py", # TODO: activate this + "src/dsp_tools/commands/project/models/project.py", # TODO: activate this + "src/dsp_tools/models/projectContext.py", # TODO: activate this + "src/dsp_tools/commands/project/models/propertyclass.py", # TODO: activate this + "src/dsp_tools/commands/project/models/resourceclass.py", # TODO: activate this + "src/dsp_tools/commands/project/models/user.py", # TODO: activate this ] @@ -167,6 +167,11 @@ ignore = [ "testdata/**" = [ "INP001", # implicit-namespace-package # there are some python files, but no __init__.py ] +"test/*" = [ + "D103", # pydocstyle: undocumented-public-function + "D102", # pydocstyle: undocumented-public-method + "D101", # pydocstyle: undocumented-public-class +] [tool.ruff.lint.pydocstyle] diff --git a/src/dsp_tools/cli/call_action.py b/src/dsp_tools/cli/call_action.py index f741f3a2d..559f5e008 100644 --- a/src/dsp_tools/cli/call_action.py +++ b/src/dsp_tools/cli/call_action.py @@ -24,7 +24,7 @@ logger = get_logger(__name__) -def call_requested_action(args: argparse.Namespace) -> bool: +def call_requested_action(args: argparse.Namespace) -> bool: # noqa: PLR0911 (Too many return statements) """ Call the appropriate method of DSP-TOOLS. diff --git a/src/dsp_tools/commands/excel2json/project.py b/src/dsp_tools/commands/excel2json/project.py index eff0422bf..a6f6f8599 100644 --- a/src/dsp_tools/commands/excel2json/project.py +++ b/src/dsp_tools/commands/excel2json/project.py @@ -57,7 +57,7 @@ def excel2json( def _validate_folder_structure_get_filenames(data_model_files: str) -> tuple[list[Path], list[Path]]: if not Path(data_model_files).is_dir(): raise UserError(f"ERROR: {data_model_files} is not a directory.") - folder = [x for x in Path(data_model_files).glob("*") if not regex.search(r"^(\.|~\$).+", x.name)] + folder = [x for x in Path(data_model_files).glob("*") if _non_hidden(x)] processed_files = [] onto_folders, processed_onto = _get_validate_onto_folder(data_model_files, folder) processed_files.extend(processed_onto) @@ -77,7 +77,7 @@ def _get_validate_list_folder(data_model_files: str, folder: list[Path]) -> tupl processed_files: list[str] = [] listfolder = [x for x in folder if x.is_dir() and x.name == "lists"] if listfolder: - listfolder_contents = [x for x in Path(listfolder[0]).glob("*") if not regex.search(r"^(\.|~\$).+", x.name)] + listfolder_contents = [x for x in Path(listfolder[0]).glob("*") if _non_hidden(x)] if not all(regex.search(r"(de|en|fr|it|rm).xlsx", file.name) for file in listfolder_contents): raise UserError( f"The only files allowed in '{data_model_files}/lists' are en.xlsx, de.xlsx, fr.xlsx, it.xlsx, rm.xlsx" @@ -94,7 +94,7 @@ def _get_validate_onto_folder(data_model_files: str, folder: list[Path]) -> tupl f"'{data_model_files}' must contain at least one subfolder named after the pattern 'onto_name (onto_label)'" ) for onto_folder in onto_folders: - contents = sorted([x.name for x in Path(onto_folder).glob("*") if not regex.search(r"^(\.|~\$).+", x.name)]) + contents = sorted([x.name for x in Path(onto_folder).glob("*") if _non_hidden(x)]) if contents != ["properties.xlsx", "resources.xlsx"]: raise UserError( f"ERROR: '{data_model_files}/{onto_folder.name}' must contain one file 'properties.xlsx' " @@ -104,6 +104,10 @@ def _get_validate_onto_folder(data_model_files: str, folder: list[Path]) -> tupl return onto_folders, processed_files +def _non_hidden(path: Path) -> bool: + return not regex.search(r"^(\.|~\$).+", path.name) + + def _create_project_json( data_model_files: str, listfolder: list[Path], onto_folders: list[Path] ) -> tuple[bool, dict[str, Any]]: diff --git a/src/dsp_tools/commands/excel2xml/excel2xml_lib.py b/src/dsp_tools/commands/excel2xml/excel2xml_lib.py index 67084bc89..74c6f052f 100644 --- a/src/dsp_tools/commands/excel2xml/excel2xml_lib.py +++ b/src/dsp_tools/commands/excel2xml/excel2xml_lib.py @@ -11,6 +11,7 @@ import regex from lxml import etree from lxml.builder import E +from regex import Match from dsp_tools.commands.excel2xml.propertyelement import PropertyElement from dsp_tools.models.exceptions import BaseError @@ -136,38 +137,43 @@ def find_date_in_string(string: str) -> Optional[str]: # sanitize input, just in case that the method was called on an empty or N/A cell if not check_notna(string): return None + try: + return _find_date_in_string_throwing(string) + except ValueError: + return None - months_dict = { - "January": 1, - "Jan": 1, - "February": 2, - "Feb": 2, - "March": 3, - "Mar": 3, - "April": 4, - "Apr": 4, - "May": 5, - "June": 6, - "Jun": 6, - "July": 7, - "Jul": 7, - "August": 8, - "Aug": 8, - "September": 9, - "Sept": 9, - "October": 10, - "Oct": 10, - "November": 11, - "Nov": 11, - "December": 12, - "Dec": 12, - } - - startdate: Optional[datetime.date] = None - enddate: Optional[datetime.date] = None - startyear: Optional[int] = None - endyear: Optional[int] = None +_months_dict = { + "January": 1, + "Jan": 1, + "February": 2, + "Feb": 2, + "March": 3, + "Mar": 3, + "April": 4, + "Apr": 4, + "May": 5, + "June": 6, + "Jun": 6, + "July": 7, + "Jul": 7, + "August": 8, + "Aug": 8, + "September": 9, + "Sept": 9, + "October": 10, + "Oct": 10, + "November": 11, + "Nov": 11, + "December": 12, + "Dec": 12, +} + + +def _find_date_in_string_throwing(string: str) -> str | None: + """ + This function is the same as find_date_in_string(), but may raise a ValueError instead of returning None. + """ year_regex = r"([0-2]?[0-9][0-9][0-9])" month_regex = r"([0-1]?[0-9])" day_regex = r"([0-3]?[0-9])" @@ -194,7 +200,7 @@ def find_date_in_string(string: str) -> Optional[str]: string, ) # template: March 9, 1908 | March5,1908 | May 11, 1906 - all_months = "|".join(months_dict) + all_months = "|".join(_months_dict) monthname_date_regex = rf"{lookbehind}({all_months}) ?{day_regex}, ?{year_regex}{lookahead}" monthname_date = regex.search(monthname_date_regex, string) # template: 1849/50 | 1849-50 | 1849/1850 @@ -202,71 +208,71 @@ def find_date_in_string(string: str) -> Optional[str]: # template: 1907 year_only = regex.search(rf"{lookbehind}{year_regex}{lookahead}", string) + res: str | None = None if iso_date: - year = int(iso_date.group(1)) - month = int(iso_date.group(2)) - day = int(iso_date.group(3)) - try: - startdate = datetime.date(year, month, day) - enddate = startdate - except ValueError: - return None - + res = _from_iso_date(iso_date) elif eur_date_range: - startday = int(eur_date_range.group(1)) - startmonth = int(eur_date_range.group(2)) if eur_date_range.group(2) else int(eur_date_range.group(5)) - startyear = int(eur_date_range.group(3)) if eur_date_range.group(3) else int(eur_date_range.group(6)) - endday = int(eur_date_range.group(4)) - endmonth = int(eur_date_range.group(5)) - endyear = int(eur_date_range.group(6)) - try: - startdate = datetime.date(startyear, startmonth, startday) - enddate = datetime.date(endyear, endmonth, endday) - if enddate < startdate: - raise ValueError - except ValueError: - return None - + res = _from_eur_date_range(eur_date_range) elif eur_date: - startday = int(eur_date.group(1)) - startmonth = int(eur_date.group(2)) - startyear = int(eur_date.group(3)) - try: - startdate = datetime.date(startyear, startmonth, startday) - enddate = startdate - except ValueError: - return None - + res = _from_eur_date(eur_date) elif monthname_date: - day = int(monthname_date.group(2)) - month = months_dict[monthname_date.group(1)] - year = int(monthname_date.group(3)) - try: - startdate = datetime.date(year, month, day) - enddate = startdate - except ValueError: - return None - + res = _from_monthname_date(monthname_date) elif year_range: - startyear = int(year_range.group(1)) - endyear = int(year_range.group(2)) - if endyear // 10 == 0: - # endyear is only 1-digit: add the first 2-3 digits of startyear - endyear = startyear // 10 * 10 + endyear - elif endyear // 100 == 0: - # endyear is only 2-digit: add the first 1-2 digits of startyear - endyear = startyear // 100 * 100 + endyear - + res = _from_year_range(year_range) elif year_only: - startyear = int(year_only.group(0)) - endyear = startyear + year = int(year_only.group(0)) + res = f"GREGORIAN:CE:{year}:CE:{year}" + return res - if startdate and enddate: - return f"GREGORIAN:CE:{startdate.isoformat()}:CE:{enddate.isoformat()}" - elif startyear and endyear: - return f"GREGORIAN:CE:{startyear}:CE:{endyear}" - else: - return None + +def _from_iso_date(iso_date: Match[str]) -> str: + year = int(iso_date.group(1)) + month = int(iso_date.group(2)) + day = int(iso_date.group(3)) + date = datetime.date(year, month, day) + return f"GREGORIAN:CE:{date.isoformat()}:CE:{date.isoformat()}" + + +def _from_eur_date_range(eur_date_range: Match[str]) -> str: + startday = int(eur_date_range.group(1)) + startmonth = int(eur_date_range.group(2)) if eur_date_range.group(2) else int(eur_date_range.group(5)) + startyear = int(eur_date_range.group(3)) if eur_date_range.group(3) else int(eur_date_range.group(6)) + endday = int(eur_date_range.group(4)) + endmonth = int(eur_date_range.group(5)) + endyear = int(eur_date_range.group(6)) + startdate = datetime.date(startyear, startmonth, startday) + enddate = datetime.date(endyear, endmonth, endday) + if enddate < startdate: + raise ValueError + return f"GREGORIAN:CE:{startdate.isoformat()}:CE:{enddate.isoformat()}" + + +def _from_eur_date(eur_date: Match[str]) -> str: + startday = int(eur_date.group(1)) + startmonth = int(eur_date.group(2)) + startyear = int(eur_date.group(3)) + date = datetime.date(startyear, startmonth, startday) + return f"GREGORIAN:CE:{date.isoformat()}:CE:{date.isoformat()}" + + +def _from_monthname_date(monthname_date: Match[str]) -> str: + day = int(monthname_date.group(2)) + month = _months_dict[monthname_date.group(1)] + year = int(monthname_date.group(3)) + date = datetime.date(year, month, day) + return f"GREGORIAN:CE:{date.isoformat()}:CE:{date.isoformat()}" + + +def _from_year_range(year_range: Match[str]) -> str: + startyear = int(year_range.group(1)) + endyear = int(year_range.group(2)) + if endyear // 10 == 0: + # endyear is only 1-digit: add the first 2-3 digits of startyear + endyear = startyear // 10 * 10 + endyear + elif endyear // 100 == 0: + # endyear is only 2-digit: add the first 1-2 digits of startyear + endyear = startyear // 100 * 100 + endyear + return f"GREGORIAN:CE:{startyear}:CE:{endyear}" def prepare_value( diff --git a/src/dsp_tools/commands/project/models/listnode.py b/src/dsp_tools/commands/project/models/listnode.py index 21f3326de..e92c1a5b9 100644 --- a/src/dsp_tools/commands/project/models/listnode.py +++ b/src/dsp_tools/commands/project/models/listnode.py @@ -35,27 +35,6 @@ from dsp_tools.utils.connection import Connection -def list_creator(con: Connection, project: Project, parent_node: "ListNode", nodes: list[dict]) -> list["ListNode"]: - nodelist: list[ListNode] = [] - - for n in nodes: - new_node = ListNode( - con=con, - project=project, - label=n["labels"], - comments=n.get("comments"), - name=n["name"], - parent=parent_node, - ) - - if n.get("nodes"): - new_node.children = list_creator(con, project, new_node, n["nodes"]) - - nodelist.append(new_node) - - return nodelist - - class ListNode(Model): """ This class represents a list node diff --git a/src/dsp_tools/commands/project/models/ontology.py b/src/dsp_tools/commands/project/models/ontology.py index 1b43c2ea4..80bb01d18 100644 --- a/src/dsp_tools/commands/project/models/ontology.py +++ b/src/dsp_tools/commands/project/models/ontology.py @@ -199,7 +199,7 @@ def fromJsonObj(cls, con: Connection, json_obj: Any) -> "Ontology": property_classes = [ PropertyClass.fromJsonObj(con=con, context=context, json_obj=a) for a in properties_obj - if WithId(a.get(knora_api + ":objectType")).str() != "knora-api:LinkValue" + if WithId(a.get(knora_api + ":objectType")).to_string() != "knora-api:LinkValue" ] return cls( con=con, diff --git a/src/dsp_tools/commands/project/models/propertyclass.py b/src/dsp_tools/commands/project/models/propertyclass.py index 7397d7969..21b1d115f 100644 --- a/src/dsp_tools/commands/project/models/propertyclass.py +++ b/src/dsp_tools/commands/project/models/propertyclass.py @@ -209,13 +209,13 @@ def fromJsonObj(cls, con: Connection, context: Context, json_obj: Any) -> "Prope superproperties = [x["@id"] for x in superproperties_obj if x and x.get("@id")] else: superproperties = None - rdf_object = WithId(json_obj.get(knora_api + ":objectType")).str() - rdf_subject = WithId(json_obj.get(knora_api + ":subjectType")).str() + rdf_object = WithId(json_obj.get(knora_api + ":objectType")).to_string() + rdf_subject = WithId(json_obj.get(knora_api + ":subjectType")).to_string() label = LangString.fromJsonLdObj(json_obj.get(rdfs + ":label")) comment = LangString.fromJsonLdObj(json_obj.get(rdfs + ":comment")) gui_element = None if json_obj.get(salsah_gui + ":guiElement") is not None: - gui_element = WithId(json_obj.get(salsah_gui + ":guiElement")).str() + gui_element = WithId(json_obj.get(salsah_gui + ":guiElement")).to_string() gui_element = gui_element.replace("Pulldown", "List") gui_element = gui_element.replace("Radio", "List") gui_attributes_list = json_obj.get(salsah_gui + ":guiAttribute") diff --git a/src/dsp_tools/commands/xmlupload/models/xmlpermission.py b/src/dsp_tools/commands/xmlupload/models/xmlpermission.py index 0d498e5bc..ab6702462 100644 --- a/src/dsp_tools/commands/xmlupload/models/xmlpermission.py +++ b/src/dsp_tools/commands/xmlupload/models/xmlpermission.py @@ -8,7 +8,7 @@ class XmlPermission: """Represents the permission set containing several XmlAllow elements in the XML used for data import""" - _id: str + permission_id: str _allows: list[XmlAllow] def __init__(self, node: etree._Element, project_context: ProjectContext) -> None: @@ -20,15 +20,10 @@ def __init__(self, node: etree._Element, project_context: ProjectContext) -> Non project_context: Context for DOM node traversal """ self._allows = [] - self._id = node.attrib["id"] + self.permission_id = node.attrib["id"] for allow_node in node: self._allows.append(XmlAllow(allow_node, project_context)) - @property - def id(self) -> str: - """The id of the permission set, p.ex. res-default""" - return self._id - def get_permission_instance(self) -> Permissions: """Returns a list of allow elements of this permission instance""" permissions = Permissions() diff --git a/src/dsp_tools/commands/xmlupload/models/xmlresource.py b/src/dsp_tools/commands/xmlupload/models/xmlresource.py index d31a90cfb..8d8e6d77e 100644 --- a/src/dsp_tools/commands/xmlupload/models/xmlresource.py +++ b/src/dsp_tools/commands/xmlupload/models/xmlresource.py @@ -28,7 +28,7 @@ class XMLResource: Represents a resource in the XML used for data import. Attributes: - id: The unique id of the resource + res_id: The unique id of the resource iri: The custom IRI of the resource ark: The custom ARK of the resource label: The label of the resource @@ -39,7 +39,7 @@ class XMLResource: properties: The list of properties of the resource """ - id: str + res_id: str iri: Optional[str] ark: Optional[str] label: str @@ -60,7 +60,7 @@ def __init__(self, node: etree._Element, default_ontology: str) -> None: Returns: None """ - self.id = node.attrib["id"] + self.res_id = node.attrib["id"] self.iri = node.attrib.get("iri") self.ark = node.attrib.get("ark") self.creation_date = None diff --git a/src/dsp_tools/commands/xmlupload/resource_create_client.py b/src/dsp_tools/commands/xmlupload/resource_create_client.py index 38c319bf9..1feaac472 100644 --- a/src/dsp_tools/commands/xmlupload/resource_create_client.py +++ b/src/dsp_tools/commands/xmlupload/resource_create_client.py @@ -37,7 +37,9 @@ def create_resource( bitstream_information: BitstreamInfo | None, ) -> tuple[str, str]: """Creates a resource on the DSP server.""" - logger.info(f"Attempting to create resource {resource.id} (label: {resource.label}, iri: {resource.iri})...") + logger.info( + f"Attempting to create resource {resource.res_id} (label: {resource.label}, iri: {resource.iri})..." + ) resource_dict = self._make_resource_with_values(resource, bitstream_information) resource_json_ld = json.dumps(resource_dict, ensure_ascii=False) res = try_network_action(self.con.post, route="/v2/resources", jsondata=resource_json_ld) @@ -79,7 +81,7 @@ def _make_resource( res["knora-api:hasPermissions"] = str(perm) else: raise BaseError( - f"Could not find permissions for resource {resource.id} with permissions {resource.permissions}" + f"Could not find permissions for resource {resource.res_id} with permissions {resource.permissions}" ) if resource.creation_date: res["knora-api:creationDate"] = { diff --git a/src/dsp_tools/commands/xmlupload/resource_multimedia.py b/src/dsp_tools/commands/xmlupload/resource_multimedia.py index a01a1dddd..d39b6c544 100644 --- a/src/dsp_tools/commands/xmlupload/resource_multimedia.py +++ b/src/dsp_tools/commands/xmlupload/resource_multimedia.py @@ -95,7 +95,7 @@ def _handle_media_upload( return resource_bitstream except BaseError as err: err_msg = err.orig_err_msg_from_api or err.message - msg = f"Unable to upload file '{bitstream.value}' of resource '{resource.label}' ({resource.id})" + msg = f"Unable to upload file '{bitstream.value}' of resource '{resource.label}' ({resource.res_id})" print(f"{datetime.now()}: WARNING: {msg}: {err_msg}") logger.warning(msg, exc_info=True) return None diff --git a/src/dsp_tools/commands/xmlupload/stash/stash_circular_references.py b/src/dsp_tools/commands/xmlupload/stash/stash_circular_references.py index b39b2d691..0a6073920 100644 --- a/src/dsp_tools/commands/xmlupload/stash/stash_circular_references.py +++ b/src/dsp_tools/commands/xmlupload/stash/stash_circular_references.py @@ -98,15 +98,15 @@ def stash_circular_references( stashed_link_values: list[LinkValueStashItem] = [] for res in resources: - if res.id not in stash_lookup: + if res.res_id not in stash_lookup: continue for link_prop in res.get_props_with_links(): assert link_prop.valtype in ["text", "resptr"] if link_prop.valtype == "text": - standoff_stash_item = _stash_standoff(res.id, res.restype, link_prop, stash_lookup) + standoff_stash_item = _stash_standoff(res.res_id, res.restype, link_prop, stash_lookup) stashed_standoff_values.extend(standoff_stash_item) elif link_prop.valtype == "resptr": - link_stash_item = _stash_resptr(res.id, res.restype, link_prop, stash_lookup, permission_lookup) + link_stash_item = _stash_resptr(res.res_id, res.restype, link_prop, stash_lookup, permission_lookup) stashed_link_values.extend(link_stash_item) if len(link_prop.values) == 0: diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 2f7d2d2f4..07672fb55 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -148,7 +148,7 @@ def _prepare_upload( root=root, default_ontology=default_ontology, ) - sorting_lookup = {res.id: res for res in resources} + sorting_lookup = {res.res_id: res for res in resources} resources = [sorting_lookup[res_id] for res_id in upload_order] logger.info("Stashing circular references...") if verbose: @@ -281,7 +281,7 @@ def _get_project_context_from_server(connection: Connection) -> ProjectContext: def _extract_permissions_from_xml(root: etree._Element, proj_context: ProjectContext) -> dict[str, XmlPermission]: permission_ele = list(root.iter(tag="permissions")) permissions = [XmlPermission(permission, proj_context) for permission in permission_ele] - return {permission.id: permission for permission in permissions} + return {permission.permission_id: permission for permission in permissions} def _extract_resources_from_xml(root: etree._Element, default_ontology: str) -> list[XMLResource]: @@ -339,18 +339,18 @@ def _upload_resources( resource, config.preprocessing_done, sipi_server, imgdir, permissions_lookup ) if not success: - failed_uploads.append(resource.id) + failed_uploads.append(resource.res_id) continue res = _create_resource(resource, media_info, resource_create_client) if not res: - failed_uploads.append(resource.id) + failed_uploads.append(resource.res_id) continue iri, label = res - id_to_iri_resolver.update(resource.id, iri) + id_to_iri_resolver.update(resource.res_id, iri) - resource_designation = f"'{label}' (ID: '{resource.id}', IRI: '{iri}')" + resource_designation = f"'{label}' (ID: '{resource.res_id}', IRI: '{iri}')" print(f"{datetime.now()}: Created resource {i+1}/{len(resources)}: {resource_designation}") logger.info(f"Created resource {i+1}/{len(resources)}: {resource_designation}") @@ -366,19 +366,19 @@ def _create_resource( return resource_create_client.create_resource(resource, bitstream_information) except BaseError as err: err_msg = err.orig_err_msg_from_api or err.message - print(f"{datetime.now()}: WARNING: Unable to create resource '{resource.label}' ({resource.id}): {err_msg}") + print(f"{datetime.now()}: WARNING: Unable to create resource '{resource.label}' ({resource.res_id}): {err_msg}") log_msg = ( - f"Unable to create resource '{resource.label}' ({resource.id})\n" + f"Unable to create resource '{resource.label}' ({resource.res_id})\n" f"Resource details:\n{vars(resource)}\n" f"Property details:\n" + "\n".join([str(vars(prop)) for prop in resource.properties]) ) logger.warning(log_msg, exc_info=True) return None except Exception as err: - msg = f"Unable to create resource '{resource.label}' ({resource.id})" + msg = f"Unable to create resource '{resource.label}' ({resource.res_id})" print(f"{datetime.now()}: WARNING: {msg}: {err}") log_msg = ( - f"Unable to create resource '{resource.label}' ({resource.id})\n" + f"Unable to create resource '{resource.label}' ({resource.res_id})\n" f"Resource details:\n{vars(resource)}\n" f"Property details:\n" + "\n".join([str(vars(prop)) for prop in resource.properties]) ) diff --git a/src/dsp_tools/models/helpers.py b/src/dsp_tools/models/helpers.py index cc2d432c1..f3258d0a1 100644 --- a/src/dsp_tools/models/helpers.py +++ b/src/dsp_tools/models/helpers.py @@ -452,5 +452,5 @@ def __init__(self, obj: Optional[dict[str, str]]): return self._tmp = obj.get("@id") - def str(self) -> Optional[str]: + def to_string(self) -> Optional[str]: return self._tmp diff --git a/src/dsp_tools/models/langstring.py b/src/dsp_tools/models/langstring.py index 78f91df5b..5e12dafa7 100644 --- a/src/dsp_tools/models/langstring.py +++ b/src/dsp_tools/models/langstring.py @@ -87,43 +87,37 @@ def mymapper(p: tuple[Union[Languages, str], str]) -> tuple[Languages, str]: raise BaseError("Not a valid language definition!") def __getitem__(self, key: Optional[Union[Languages, str]] = None) -> Optional[str]: - # - # First deal with simple strings (no language given). We return, if existing, the simple string - # or just the first language dependent string - # - if key is None: - if self._simplestring: - return self._simplestring - elif len(self._langstrs) != 0: - return next(iter(self._langstrs)) - else: - return None - else: - pass - # self._simplestring = None # Let's delete the string without language if there is one... - if isinstance(key, Enum): - if self._langstrs.get(key) is None: - for lst in self._langstrs: - if self._langstrs.get(lst) is not None: - return self._langstrs[lst] - if self._simplestring is not None: - return self._simplestring - return None - else: - return self._langstrs[key] + match key: + case None: + return self.__getitem_nokey__() + case Enum() if ls := self._langstrs.get(key): + return ls + case Enum(): + return self.__getitem_fallback__() + case str(): + lmap = {a.value: a for a in Languages} + lkey = lmap.get(key.lower()) + if lkey is None: + raise BaseError(f"Invalid language string '{key}'") + if ls := self._langstrs.get(lkey): + return ls + return self.__getitem_fallback__() + + def __getitem_nokey__(self) -> Optional[str]: + if self._simplestring: + return self._simplestring + elif self._langstrs: + return next(iter(self._langstrs)).value else: - lmap = dict(map(lambda a: (a.value, a), Languages)) - if lmap.get(key.lower()) is None: - raise BaseError('Invalid language string "' + key + '"!') - if self._langstrs.get(lmap[key.lower()]) is None: - for lst in self._langstrs: - if self._langstrs.get(lst) is not None: - return self._langstrs[lst] - if self._simplestring is not None: - return self._simplestring - return None - else: - return self._langstrs[lmap[key.lower()]] + return None + + def __getitem_fallback__(self) -> Optional[str]: + for lst in self._langstrs: + if self._langstrs.get(lst) is not None: + return self._langstrs[lst] + if self._simplestring is not None: + return self._simplestring + return None def __setitem__(self, key: Optional[Union[Languages, str]], value: str): if key is None: diff --git a/test/unittests/commands/excel2json/test_lists.py b/test/unittests/commands/excel2json/test_lists.py index 5318c6416..41d5d359f 100644 --- a/test/unittests/commands/excel2json/test_lists.py +++ b/test/unittests/commands/excel2json/test_lists.py @@ -54,11 +54,9 @@ def test_excel2lists_invalid_excel2() -> None: class TestValidateListSection: def test_correct(self) -> None: - """validate the valid "lists" section: should not raise an error""" assert e2l.validate_lists_section_with_schema(lists_section=lists_section_valid) is True def test_without_comments(self) -> None: - """remove mandatory "comments" section from root node: should raise an error""" with open("testdata/excel2json/lists-multilingual-output-expected.json", encoding="utf-8") as f: lists_section_valid_invalid = json.load(f) del lists_section_valid_invalid[0]["comments"] @@ -69,7 +67,6 @@ def test_without_comments(self) -> None: e2l.validate_lists_section_with_schema(lists_section=lists_section_valid_invalid) def test_invalid_lang(self) -> None: - """insert invalid language code in "comments" section: should raise an error""" lists_section_valid[0]["comments"]["eng"] = "wrong English label" expected_msg = re.escape( @@ -79,13 +76,11 @@ def test_invalid_lang(self) -> None: e2l.validate_lists_section_with_schema(lists_section=lists_section_valid) def test_wrong_signature_wrong_data(self) -> None: - """wrong usage of the function: should raise an error""" expected_msg = re.escape("works only if exactly one of the two arguments is given") with pytest.raises(BaseError, match=expected_msg): e2l.validate_lists_section_with_schema() def test_wrong_signature_no_data(self) -> None: - """wrong usage of the function: should raise an error""" expected_msg = r"works only if exactly one of the two arguments is given" with pytest.raises(BaseError, match=expected_msg): e2l.validate_lists_section_with_schema( @@ -94,7 +89,6 @@ def test_wrong_signature_no_data(self) -> None: ) def test_file_without_list(self) -> None: - """pass a file that doesn't have a "lists" section""" tp_minimal = "testdata/json-project/test-project-minimal.json" expected_msg = r"there is no 'lists' section" with pytest.raises(BaseError, match=expected_msg): diff --git a/test/unittests/commands/excel2json/test_project.py b/test/unittests/commands/excel2json/test_project.py index d79be380e..e50a48804 100644 --- a/test/unittests/commands/excel2json/test_project.py +++ b/test/unittests/commands/excel2json/test_project.py @@ -7,8 +7,6 @@ class TestCreateProject(unittest.TestCase): - """Test the create project command""" - def test_excel_to_json_project(self) -> None: excel_folder = "testdata/excel2json/excel2json_files" listfolder, onto_folders = _validate_folder_structure_get_filenames(excel_folder) diff --git a/test/unittests/commands/excel2xml/test_excel2xml_lib.py b/test/unittests/commands/excel2xml/test_excel2xml_lib.py index 5c704ea83..a94bdb6ff 100644 --- a/test/unittests/commands/excel2xml/test_excel2xml_lib.py +++ b/test/unittests/commands/excel2xml/test_excel2xml_lib.py @@ -630,51 +630,104 @@ def test_make_uri_prop(self) -> None: invalid_values = ["https:", 10.0, 5, "www.test.com"] run_test(self, prop, method, different_values, invalid_values) - def test_make_annotation_link_region(self) -> None: - """ - This method tests 3 methods at the same time: make_annotation(), make_link(), and make_region(). - """ - method_2_tagname: dict[Callable[..., etree._Element], str] = { - excel2xml.make_annotation: "annotation", - excel2xml.make_link: "link", - excel2xml.make_region: "region", - } - for method, tagname in method_2_tagname.items(): - test_cases: list[tuple[Callable[..., etree._Element], str]] = [ - ( - lambda: method("label", "id"), - f'<{tagname} label="label" id="id" permissions="res-default"/>', - ), - ( - lambda: method("label", "id", "res-restricted"), - f'<{tagname} label="label" id="id" permissions="res-restricted"/>', - ), - ( - lambda: method("label", "id", ark="ark"), - f'<{tagname} label="label" id="id" permissions="res-default" ark="ark"/>', - ), - ( - lambda: method("label", "id", iri="iri"), - f'<{tagname} label="label" id="id" permissions="res-default" iri="iri"/>', - ), - ( - lambda: method("label", "id", creation_date="2019-10-23T13:45:12Z"), - ( - f'<{tagname} label="label" id="id" permissions="res-default" ' - 'creation_date="2019-10-23T13:45:12Z"/>' - ), - ), - ] - for _method, result in test_cases: - xml_returned_as_element = _method() - xml_returned = etree.tostring(xml_returned_as_element, encoding="unicode") - xml_returned = regex.sub(r" xmlns(:.+?)?=\".+?\"", "", xml_returned) - self.assertEqual(result, xml_returned) - - with self.assertWarns(UserWarning): - method("label", "id", ark="ark", iri="iri") - with self.assertRaisesRegex(BaseError, "invalid creation date"): - method("label", "restype", "id", creation_date="2019-10-23T13:45:12") + def test_make_annotation(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_annotation("label", "id")) + self.assertEqual(expected, result) + + def test_make_annotation_with_permission(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_annotation("label", "id", "res-restricted")) + self.assertEqual(expected, result) + + def test_make_annotation_with_ark(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_annotation("label", "id", ark="ark")) + self.assertEqual(expected, result) + + def test_make_annotation_with_iri(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_annotation("label", "id", iri="iri")) + self.assertEqual(expected, result) + + def test_make_annotation_with_creation_date(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_annotation("label", "id", creation_date="2019-10-23T13:45:12Z")) + self.assertEqual(expected, result) + + def test_warn_make_annotation_with_iri_and_ark(self) -> None: + with self.assertWarns(UserWarning): + excel2xml.make_annotation("label", "id", ark="ark", iri="iri") + + def test_fail_annotation_with_invalid_creation_date(self) -> None: + with self.assertRaisesRegex(BaseError, "invalid creation date"): + excel2xml.make_annotation("label", "id", creation_date="2019-10-23T13:45:12") + + def test_make_link(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_link("label", "id")) + self.assertEqual(expected, result) + + def test_make_link_with_permission(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_link("label", "id", "res-restricted")) + self.assertEqual(expected, result) + + def test_make_link_with_ark(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_link("label", "id", ark="ark")) + self.assertEqual(expected, result) + + def test_make_link_with_iri(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_link("label", "id", iri="iri")) + self.assertEqual(expected, result) + + def test_make_link_with_creation_date(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_link("label", "id", creation_date="2019-10-23T13:45:12Z")) + self.assertEqual(expected, result) + + def test_warn_make_link_with_iri_and_ark(self) -> None: + with self.assertWarns(UserWarning): + excel2xml.make_link("label", "id", ark="ark", iri="iri") + + def test_fail_link_with_invalid_creation_date(self) -> None: + with self.assertRaisesRegex(BaseError, "invalid creation date"): + excel2xml.make_link("label", "id", creation_date="2019-10-23T13:45:12") + + def test_make_region(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_region("label", "id")) + self.assertEqual(expected, result) + + def test_make_region_with_permission(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_region("label", "id", "res-restricted")) + self.assertEqual(expected, result) + + def test_make_region_with_ark(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_region("label", "id", ark="ark")) + self.assertEqual(expected, result) + + def test_make_region_with_iri(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_region("label", "id", iri="iri")) + self.assertEqual(expected, result) + + def test_make_region_with_creation_date(self) -> None: + expected = '' + result = _strip_namespace(excel2xml.make_region("label", "id", creation_date="2019-10-23T13:45:12Z")) + self.assertEqual(expected, result) + + def test_warn_make_region_with_iri_and_ark(self) -> None: + with self.assertWarns(UserWarning): + excel2xml.make_region("label", "id", ark="ark", iri="iri") + + def test_fail_region_with_invalid_creation_date(self) -> None: + with self.assertRaisesRegex(BaseError, "invalid creation date"): + excel2xml.make_region("label", "id", creation_date="2019-10-23T13:45:12") def test_make_resource(self) -> None: test_cases: list[tuple[Callable[..., etree._Element], str]] = [ @@ -763,5 +816,12 @@ def test_create_json_list_mapping(self) -> None: self.assertDictEqual(testlist_mapping_returned, testlist_mapping_expected) +def _strip_namespace(element: etree._Element) -> str: + """Removes the namespace from the XML element.""" + xml = etree.tostring(element, encoding="unicode") + xml = regex.sub(r" xmlns(:.+?)?=\".+?\"", "", xml) + return xml + + if __name__ == "__main__": pytest.main([__file__]) diff --git a/test/unittests/commands/xmlupload/test_list_client_live.py b/test/unittests/commands/xmlupload/test_list_client_live.py index 799f13781..364698da8 100644 --- a/test/unittests/commands/xmlupload/test_list_client_live.py +++ b/test/unittests/commands/xmlupload/test_list_client_live.py @@ -24,8 +24,6 @@ def get( class TestGetListNodeLookup: - """Test the get_list_node_id_to_iri_lookup method of the ListClientLive class.""" - def test_list_root_only(self) -> None: list_iris = {"lists": [{"id": "http://www.example.org/lists#a"}]} list_a = { @@ -121,8 +119,6 @@ def test_multiple_lists(self) -> None: class TestGetListFromServer: - """Test the _get_list_info_from_server function.""" - def test_no_name_no_children(self) -> None: list_response = { "list": {