Skip to content

Commit 115a8b9

Browse files
committed
refactor: Improve code readability and fix minor issues in HoldingsCsvTransformer
1 parent a79e9c3 commit 115a8b9

1 file changed

Lines changed: 24 additions & 16 deletions

File tree

src/folio_migration_tools/migration_tasks/holdings_csv_transformer.py

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,8 @@ class TaskConfiguration(AbstractTaskConfiguration):
166166
title="Statistical code map file name",
167167
description=(
168168
"Path to the file containing the mapping of statistical codes. "
169-
"The file should be in TSV format with legacy_stat_code and folio_code columns."
169+
"The file should be in TSV format with legacy_stat_code and "
170+
"folio_code columns."
170171
),
171172
),
172173
] = ""
@@ -182,6 +183,7 @@ def __init__(
182183
folio_client,
183184
use_logging: bool = True,
184185
):
186+
self.task_configuration = task_config
185187
super().__init__(library_config, task_config, folio_client, use_logging)
186188
self.fallback_holdings_type = None
187189
self.folio_keys, self.holdings_field_map = self.load_mapped_fields()
@@ -214,7 +216,7 @@ def __init__(
214216
self.folio_client.folio_get_all("/holdings-types", "holdingsTypes")
215217
)
216218
logging.info("%s\tholdings types in tenant", len(self.holdings_types))
217-
self.validate_merge_criterias()
219+
self.validate_merge_criteria()
218220
self.check_source_files(
219221
self.folder_structure.data_folder / "items", self.task_configuration.files
220222
)
@@ -282,28 +284,31 @@ def load_call_number_type_map(self):
282284
self.folder_structure.mapping_files_folder
283285
/ self.task_configuration.call_number_type_map_file_name,
284286
"r",
285-
) as callnumber_type_map_f:
287+
) as call_number_type_map_f:
286288
return self.load_ref_data_map_from_file(
287-
callnumber_type_map_f, "Found %s rows in call number type map"
289+
call_number_type_map_f, "Found %s rows in call number type map"
288290
)
289291

290292
def load_location_map(self):
291293
with open(
292-
self.folder_structure.mapping_files_folder / self.task_configuration.location_map_file_name
294+
self.folder_structure.mapping_files_folder /
295+
self.task_configuration.location_map_file_name
293296
) as location_map_f:
294297
return self.load_ref_data_map_from_file(
295298
location_map_f, "Found %s rows in location map"
296299
)
297300

298301
# TODO Rename this here and in `load_call_number_type_map` and `load_location_map`
299-
def load_ref_data_map_from_file(self, file, message):
302+
@staticmethod
303+
def load_ref_data_map_from_file(file, message):
300304
ref_dat_map = list(csv.DictReader(file, dialect="tsv"))
301305
logging.info(message, len(ref_dat_map))
302306
return ref_dat_map
303307

304308
def load_mapped_fields(self):
305309
with open(
306-
self.folder_structure.mapping_files_folder / self.task_configuration.holdings_map_file_name
310+
self.folder_structure.mapping_files_folder
311+
/ self.task_configuration.holdings_map_file_name
307312
) as holdings_mapper_f:
308313
holdings_map = json.load(holdings_mapper_f)
309314
logging.info("%s fields in holdings mapping file map", len(holdings_map["data"]))
@@ -331,7 +336,8 @@ def do_work(self):
331336
print(f"\n{error_str}\nHalting")
332337
sys.exit(1)
333338
logging.info(
334-
f"processed {self.total_records:,} records in {len(self.task_configuration.files)} files"
339+
f"processed {self.total_records:,} records in "
340+
f"{len(self.task_configuration.files)} files"
335341
)
336342

337343
def wrap_up(self):
@@ -373,16 +379,18 @@ def wrap_up(self):
373379
logging.info("All done!")
374380
self.clean_out_empty_logs()
375381

376-
def validate_merge_criterias(self):
382+
def validate_merge_criteria(self):
377383
holdings_schema = self.folio_client.get_holdings_schema()
378384
properties = holdings_schema["properties"].keys()
379385
logging.info(properties)
380386
logging.info(self.task_configuration.holdings_merge_criteria)
381-
res = [mc for mc in self.task_configuration.holdings_merge_criteria if mc not in properties]
387+
res = [
388+
mc for mc in self.task_configuration.holdings_merge_criteria if mc not in properties
389+
]
382390
if any(res):
383391
logging.critical(
384392
(
385-
"Merge criteria(s) is not a property of a holdingsrecord: %s"
393+
"Merge criteria is not a property of a holdings record: %s"
386394
"check the merge criteria names and try again"
387395
),
388396
", ".join(res),
@@ -409,8 +417,8 @@ def process_single_file(self, file_def: FileDefinition):
409417
self.mapper.handle_transformation_process_error(idx, process_error)
410418
except TransformationRecordFailedError as error:
411419
self.mapper.handle_transformation_record_failed_error(idx, error)
412-
except Exception as excepion:
413-
self.mapper.handle_generic_exception(idx, excepion)
420+
except Exception as e:
421+
self.mapper.handle_generic_exception(idx, e)
414422
self.mapper.migration_report.add_general_statistics(
415423
i18n.t("Number of Legacy items in file")
416424
)
@@ -461,14 +469,14 @@ def create_bound_with_holdings(self, folio_holding, legacy_id: str):
461469
def merge_holding_in(
462470
self, incoming_holding: dict, instance_ids: list[str], legacy_item_id: str
463471
) -> None:
464-
"""Determines what newly generated holdingsrecords are to be merged with
472+
"""Determines what newly generated holdings records are to be merged with
465473
previously created ones. When that is done, it generates the correct boundwith
466474
parts needed.
467475
468476
Args:
469477
incoming_holding (dict): The newly created FOLIO Holding
470478
instance_ids (list): the instance IDs tied to the current item
471-
legacy_item_id (str): Id of the Item the holding was generated from
479+
legacy_item_id (str): ID of the Item the holding was generated from
472480
"""
473481
if len(instance_ids) > 1:
474482
# Is boundwith
@@ -495,7 +503,7 @@ def merge_holding_in(
495503
i18n.t("BW Items found tied to previously created BW Holdings")
496504
)
497505
else:
498-
# Regular holding. Merge according to criteria
506+
# Regular holding. Merge, according to criteria
499507
new_holding_key = HoldingsHelper.to_key(
500508
incoming_holding,
501509
self.task_configuration.holdings_merge_criteria,

0 commit comments

Comments
 (0)