From 97066a77b5f47004333e9a3d09ed7c7f6a16b677 Mon Sep 17 00:00:00 2001 From: Faad Abubakar Date: Tue, 7 Apr 2026 20:50:57 +0100 Subject: [PATCH] refactor: extract acro_artifacts folder name to ARTIFACTS_DIR constant --- acro/acro_tables.py | 39 ++++++++++++++++++++------------------- acro/constants.py | 3 +++ acro/record.py | 7 ++++--- test/test_initial.py | 15 ++++++++------- 4 files changed, 35 insertions(+), 29 deletions(-) create mode 100644 acro/constants.py diff --git a/acro/acro_tables.py b/acro/acro_tables.py index 72ecbe2..6bb53e0 100644 --- a/acro/acro_tables.py +++ b/acro/acro_tables.py @@ -16,6 +16,7 @@ from pandas import DataFrame, Series from . import utils +from .constants import ARTIFACTS_DIR from .record import Records logger = logging.getLogger("acro") @@ -584,10 +585,10 @@ def survival_plot( plot = survival_func.plot() try: - os.makedirs("acro_artifacts") - logger.debug("Directory acro_artifacts created successfully") + os.makedirs(ARTIFACTS_DIR) + logger.debug("Directory %s created successfully", ARTIFACTS_DIR) except FileExistsError: # pragma: no cover - logger.debug("Directory acro_artifacts already exists") + logger.debug("Directory %s already exists", ARTIFACTS_DIR) # create a unique filename with number to avoid overwrite filename, extension = os.path.splitext(filename) @@ -596,10 +597,10 @@ def survival_plot( return None # pragma: no cover increment_number = 0 while os.path.exists( - f"acro_artifacts/{filename}_{increment_number}{extension}" + f"{ARTIFACTS_DIR}/{filename}_{increment_number}{extension}" ): # pragma: no cover increment_number += 1 - unique_filename = f"acro_artifacts/{filename}_{increment_number}{extension}" + unique_filename = f"{ARTIFACTS_DIR}/{filename}_{increment_number}{extension}" # save the plot to the acro artifacts directory plt.savefig(unique_filename) @@ -776,12 +777,12 @@ def hist( f"The maximum value of the {column} column is: {max_value}" ) - # create the acro_artifacts directory to save the plot in it + # create the artifacts directory to save the plot in it try: - os.makedirs("acro_artifacts") - logger.debug("Directory acro_artifacts created successfully") + os.makedirs(ARTIFACTS_DIR) + logger.debug("Directory %s created successfully", ARTIFACTS_DIR) except FileExistsError: # pragma: no cover - logger.debug("Directory acro_artifacts already exists") + logger.debug("Directory %s already exists", ARTIFACTS_DIR) # create a unique filename with number to avoid overwrite filename, extension = os.path.splitext(filename) @@ -790,10 +791,10 @@ def hist( return None increment_number = 0 while os.path.exists( - f"acro_artifacts/{filename}_{increment_number}{extension}" + f"{ARTIFACTS_DIR}/{filename}_{increment_number}{extension}" ): # pragma: no cover increment_number += 1 - unique_filename = f"acro_artifacts/{filename}_{increment_number}{extension}" + unique_filename = f"{ARTIFACTS_DIR}/{filename}_{increment_number}{extension}" # save the plot to the acro artifacts directory plt.savefig(unique_filename) @@ -826,7 +827,7 @@ def pie( suppress=True. Otherwise the chart is produced and marked as "review". - The chart is saved to acro_artifacts/ with a unique incrementing + The chart is saved to the artifacts directory with a unique incrementing number appended to avoid overwriting existing files. Parameters @@ -875,12 +876,12 @@ def pie( # CREATE SUMMARY summary = f"Pie chart of {column}. Categories and counts: {counts.to_dict()}." - # CREATE acro_artifacts DIRECTORY to save plot in + # CREATE artifacts DIRECTORY to save plot in try: - os.makedirs("acro_artifacts") - logger.debug("Directory acro_artifacts created successfully") + os.makedirs(ARTIFACTS_DIR) + logger.debug("Directory %s created successfully", ARTIFACTS_DIR) except FileExistsError: # pragma: no cover - logger.debug("Directory acro_artifacts already exists") + logger.debug("Directory %s already exists", ARTIFACTS_DIR) # CREATE UNIQUE FILENAME to avoid overwrite @@ -891,12 +892,12 @@ def pie( increment_number = 0 while os.path.exists( - f"acro_artifacts/{filename}_{increment_number}{extension}" + f"{ARTIFACTS_DIR}/{filename}_{increment_number}{extension}" ): # pragma: no cover increment_number += 1 - unique_filename = f"acro_artifacts/{filename}_{increment_number}{extension}" + unique_filename = f"{ARTIFACTS_DIR}/{filename}_{increment_number}{extension}" - # SAVE PLOT to acro_artifacts directory + # SAVE PLOT to artifacts directory plt.savefig(unique_filename) # RECORD OUTPUT diff --git a/acro/constants.py b/acro/constants.py new file mode 100644 index 0000000..4757141 --- /dev/null +++ b/acro/constants.py @@ -0,0 +1,3 @@ +"""ACRO: Global constants.""" + +ARTIFACTS_DIR = "acro_artifacts" diff --git a/acro/record.py b/acro/record.py index 1868cfe..05aeb3e 100644 --- a/acro/record.py +++ b/acro/record.py @@ -14,6 +14,7 @@ import pandas as pd from pandas import DataFrame +from .constants import ARTIFACTS_DIR from .version import __version__ logger = logging.getLogger("acro:records") @@ -452,9 +453,9 @@ def finalise(self, path: str, ext: str, interactive: bool = False) -> None: else: raise ValueError("Invalid file extension. Options: {json, xlsx}") self.write_checksums(path) - # check if the directory acro_artifacts exists and delete it - if os.path.exists("acro_artifacts"): - shutil.rmtree("acro_artifacts") + # check if the artifacts directory exists and delete it + if os.path.exists(ARTIFACTS_DIR): + shutil.rmtree(ARTIFACTS_DIR) logger.info("outputs written to: %s", path) def finalise_json(self, path: str) -> None: diff --git a/test/test_initial.py b/test/test_initial.py index a9af6cb..3bd9637 100644 --- a/test/test_initial.py +++ b/test/test_initial.py @@ -12,6 +12,7 @@ from acro import ACRO, acro_tables, add_constant, add_to_acro, record, utils from acro.acro_tables import _rounded_survival_table, crosstab_with_totals +from acro.constants import ARTIFACTS_DIR from acro.record import Records, load_records PATH: str = "RES_PYTEST" @@ -705,7 +706,7 @@ def test_surv_func(acro): assert "cells suppressed" in output.summary # plot - filename = os.path.normpath("acro_artifacts/kaplan-meier_0.png") + filename = os.path.normpath(f"{ARTIFACTS_DIR}/kaplan-meier_0.png") _ = acro.surv_func(data.futime, data.death, output="plot") assert os.path.exists(filename) acro.add_exception("output_0", "I need this") @@ -1075,7 +1076,7 @@ def test_crosstab_with_manual_totals_with_suppression_with_two_aggfunc( def test_histogram_disclosive(data, acro, caplog): """Test a discolsive histogram.""" - filename = os.path.normpath("acro_artifacts/histogram_0.png") + filename = os.path.normpath(f"{ARTIFACTS_DIR}/histogram_0.png") _ = acro.hist(data, "inc_grants") assert os.path.exists(filename) acro.add_exception("output_0", "Let me have it") @@ -1092,7 +1093,7 @@ def test_histogram_disclosive(data, acro, caplog): def test_histogram_non_disclosive(data, acro): """Test a non disclosive histogram.""" - filename = os.path.normpath("acro_artifacts/histogram_0.png") + filename = os.path.normpath(f"{ARTIFACTS_DIR}/histogram_0.png") _ = acro.hist(data, "inc_grants", bins=1) assert os.path.exists(filename) acro.add_exception("output_0", "Let me have it") @@ -1105,14 +1106,14 @@ def test_histogram_non_disclosive(data, acro): def test_pie_disclosive(acro, caplog): """Test a disclosive pie chart (a category has fewer than threshold observations).""" - shutil.rmtree("acro_artifacts", ignore_errors=True) + shutil.rmtree(ARTIFACTS_DIR, ignore_errors=True) shutil.rmtree(PATH, ignore_errors=True) df = pd.DataFrame( {"grant_type": (["A"] * 20) + (["B"] * 15) + (["C"] * 12) + (["D"] * 5)} ) - filename = os.path.normpath("acro_artifacts/pie_0.png") + filename = os.path.normpath(f"{ARTIFACTS_DIR}/pie_0.png") _ = acro.pie(df, "grant_type", filename="pie.png") assert os.path.exists(filename) @@ -1131,9 +1132,9 @@ def test_pie_disclosive(acro, caplog): def test_pie_non_disclosive(data, acro): """Test a non-disclosive pie chart (all categories meet the threshold).""" - shutil.rmtree("acro_artifacts", ignore_errors=True) + shutil.rmtree(ARTIFACTS_DIR, ignore_errors=True) shutil.rmtree(PATH, ignore_errors=True) - filename = os.path.normpath("acro_artifacts/pie_0.png") + filename = os.path.normpath(f"{ARTIFACTS_DIR}/pie_0.png") result = acro.pie(data, "grant_type", filename="pie.png") assert os.path.normpath(result) == filename assert os.path.exists(filename)