fix: accept multipath in the site cache functions - #544
Open
darrenhuai wants to merge 3 commits into
Open
Conversation
Four site directories change with multipath: site_data_dir and site_config_dir on Unix and macOS, site_applications_dir on both, and site_cache_dir on macOS when the interpreter comes from Homebrew. The module-level functions expose the argument for the first three, but site_cache_dir() and site_cache_path() take no multipath at all, so passing it is a TypeError and the Homebrew cache prefix is unreachable from the function API. explanation.rst already documents the behaviour those functions cannot produce. Adding it keyword-only keeps opinion and ensure_exists where they are for positional callers, and matches how the applications functions took their new options. On site_cache_path it is a no-op, since the path variants return the first entry either way. Kept for the same reason site_data_path and site_config_path carry it: test_function_interface_is_in_sync requires the dir and path signatures to match. The new test parametrizes over the whole multipath family rather than just the cache case, so the next site directory that grows a multipath variant has to be reachable from its function too. Also corrects the multipath docstring in api.py, which still claimed only site_data_dir and site_config_dir were affected.
for more information, see https://pre-commit.ci
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
multipathchanges four site directories:site_data_dirandsite_config_diron Unix and macOS,site_applications_diron both, andsite_cache_diron macOS when the interpreter comes from Homebrew. The module-level functions expose the argument for the first three, butsite_cache_dir()andsite_cache_path()take nomultipathat all, so passing it is aTypeErrorand the Homebrew cache prefix is unreachable from the function API:docs/explanation.rstalready documents the behaviour those functions cannot produce ("site_data_dirandsite_cache_dirinclude the Homebrew prefix as an additional path whenmultipath=True").Adding it keyword-only keeps
opinionandensure_existswhere they are for positional callers, and matches how the applications functions took their new options in #534/#535.On
site_cache_pathit is a no-op, since the path variants return the first entry either way. I kept it for the same reasonsite_data_pathandsite_config_pathcarry it —test_function_interface_is_in_syncrequires the dir and path signatures to match.The new test parametrizes over the whole multipath family rather than just the cache case, so the next site directory that grows a multipath variant has to be reachable from its function too. Also corrected the
multipathdocstring inapi.py, which still claimed onlysite_data_dirandsite_config_dirwere affected.Impact is narrow, and worth stating plainly: it only bites a macOS user on Homebrew Python who wants both cache roots and is using the function API rather than
PlatformDirs. It is an API-completeness gap, not a crash anyone has reported.Verified locally on Windows: full suite 1249 passed / 94 skipped,
ruff check src tests,ruff format --check,ty check --error-on-warning .all clean, and the guard test fails both when the change is reverted and when the signature is kept but the argument is not forwarded. I did not build the docs (no sphinx in my venv); the docstring edit is a plain literal.