Skip to content

RFC: deduplicate implementations for get_config_dir and get_cache_dir #17188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pllim merged 1 commit into from
Oct 17, 2024
Merged

RFC: deduplicate implementations for get_config_dir and get_cache_dir #17188

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 27 additions & 45 deletions astropy/config/paths.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,31 @@
]


def _get_dir_path(rootname: str, cls: type, fallback: str) -> Path:
# If using set_temp_x, that overrides all
if (xch := cls._temp_path) is not None:
path = xch / rootname
if not path.is_file():

@neutrinoceros neutrinoceros Oct 16, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that this condition didn't exist in get_config_dir, but this implementation seems to pass all tests (while unconditionally calling path.mkdir doesn't), so I figured it's preferable to special casing cache.

path.mkdir(exist_ok=True)
return path.resolve()

# first look for XDG_CONFIG_HOME
if (
(xdg_config_home := os.getenv("XDG_CONFIG_HOME")) is not None
and (xch := Path(xdg_config_home)).exists()
and not (xchpth := xch / rootname).is_symlink()
):
if xchpth.exists():
return xchpth.resolve()

# symlink will be set to this if the directory is created
linkto = xchpth
else:
linkto = None
Comment thread
mhvk marked this conversation as resolved.
Resolved

return _find_or_create_root_dir(fallback, linkto, rootname)


def get_config_dir_path(rootname: str = "astropy") -> Path:
"""
Determines the package configuration directory name and creates the
Expand All @@ -42,28 +67,7 @@ def get_config_dir_path(rootname: str = "astropy") -> Path:
The absolute path to the configuration directory.

"""
# symlink will be set to this if the directory is created
linkto = None

# If using set_temp_config, that overrides all
if set_temp_config._temp_path is not None:
xch = set_temp_config._temp_path
config_path = xch / rootname
config_path.mkdir(exist_ok=True)
return config_path.resolve()

# first look for XDG_CONFIG_HOME
if (
(xdg_config_home := os.getenv("XDG_CONFIG_HOME")) is not None
and (xch := Path(xdg_config_home)).exists()
and not (xchpth := xch / rootname).is_symlink()
):
if xchpth.exists():
return xchpth.resolve()
else:
linkto = xchpth

return _find_or_create_root_dir("config", linkto, rootname)
return _get_dir_path(rootname, set_temp_config, "config")


def get_config_dir(rootname: str = "astropy") -> str:
Expand Down Expand Up @@ -104,29 +108,7 @@ def get_cache_dir_path(rootname: str = "astropy") -> Path:
The absolute path to the cache directory.

"""
# symlink will be set to this if the directory is created
linkto = None

# If using set_temp_cache, that overrides all
if set_temp_cache._temp_path is not None:
xch = set_temp_cache._temp_path
cache_path = xch / rootname
if not cache_path.is_file():
cache_path.mkdir(exist_ok=True)
return cache_path.resolve()

# first look for XDG_CACHE_HOME
if (
(xdg_config_home := os.getenv("XDG_CONFIG_HOME")) is not None

@saimn saimn Dec 6, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should have been XDG_CACHE_HOME here I think, so currently the code is using only XDG_CONFIG_HOME I think, not XDG_CACHE_HOME.

@neutrinoceros neutrinoceros Dec 10, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. For posterity this is being addressed in #17514

and (xch := Path(xdg_config_home)).exists()
and not (xchpth := xch / rootname).is_symlink()
):
if xchpth.exists():
return xchpth.resolve()
else:
linkto = xchpth

return _find_or_create_root_dir("cache", linkto, rootname)
return _get_dir_path(rootname, set_temp_cache, "cache")


def get_cache_dir(rootname: str = "astropy") -> str:
Expand Down