Skip to content

Consider removing Config.config_path, a redundant read-only alias for configpath #207

Description

@dk107dk

Config has two properties pointing at the same underlying state:

@property
def configpath(self) -> str:
    ...

@configpath.setter
def configpath(self, path: str) -> None:
    # setting this triggers reload logic, cycle detection, etc.
    ...

@property
def config_path(self) -> str:
    return self.configpath

config_path is read-only (no setter) and just proxies to configpath, which is the real, actively-used member throughout the class (internally Config always uses self.configpath / self._configpath, never self.config_path).

Checked every caller in the codebase (grep across csvpath/ and tests/): config_path has exactly 4 read-only callers total (tests/conftest.py, csvpath/cli/debug_config.py, and two test files), none of which rely on it being distinct from configpath.

David, reviewing PR #203: 'config_path() is rarely if ever used. we always use configpath. having a property that aliases such an ingrained direct member is asking for silly problems some day.' Concretely: since config_path has no setter, config.config_path = X raises AttributeError, while config.configpath = X works and triggers real reload behavior -- two names for the same thing that do not behave the same way if you guess wrong.

Recommendation: remove config_path and point the few existing callers at configpath directly. Left a TODO comment in csvpath/util/config.py pointing at this issue rather than making the change now, per tests-only scope.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions