Skip to content

refactor: extract acro_artifacts folder name to ARTIFACTS_DIR constant#376

Open
Faadabu wants to merge 1 commit intoAI-SDC:mainfrom
Faadabu:refactor/acro-artifacts-constant
Open

refactor: extract acro_artifacts folder name to ARTIFACTS_DIR constant#376
Faadabu wants to merge 1 commit intoAI-SDC:mainfrom
Faadabu:refactor/acro-artifacts-constant

Conversation

@Faadabu
Copy link
Copy Markdown

@Faadabu Faadabu commented Apr 7, 2026

Summary

Replaces all hardcoded "acro_artifacts" strings with a global ARTIFACTS_DIR constant, improving code maintainability.

Changes

  • acro/constants.py (new): Defines ARTIFACTS_DIR = "acro_artifacts"
  • acro/acro_tables.py: Replaced 15 occurrences across surv_func, hist, pie
  • acro/record.py: Replaced 2 occurrences in finalise
  • test/test_initial.py: Replaced 7 occurrences across plot/histogram/pie tests

Why constants.py instead of __init__.py?

A previous attempt (#361) placed the constant in __init__.py, which caused circular imports. A standalone constants.py with no internal package imports avoids this.

Testing

  • All 62 tests pass (pytest test/test_initial.py)
  • Pre-commit hooks pass (ruff, mypy, codespell, etc.)

Fixes #358

@Faadabu
Copy link
Copy Markdown
Author

Faadabu commented Apr 7, 2026

@rpreen Hi, this PR fixes #358 and avoids the circular import issue that caused #361 to be closed. I would appreciate a review when you have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Refactor] acro_artifacts folder is hard coded all over the place

1 participant