Skip to content

Replace deprecated appdirs with platformdirs, fix config path lookup with non conda-based environments #113

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
merged 13 commits into from
Jan 27, 2025
Merged

Replace deprecated appdirs with platformdirs, fix config path lookup with non conda-based environments #113

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
bf4dcf8
Replace `appdirs` with `platformdirs`
agriyakhetarpal Jan 24, 2025
3e3fc97
Use `user_cache_dir` from `platformdirs`
agriyakhetarpal Jan 24, 2025
e66b5fa
Set `DEFAULT_CONFIG_PATH` to `user_config_dir`
agriyakhetarpal Jan 24, 2025
8739917
Move `empack_config.yaml` to inside the project
agriyakhetarpal Jan 24, 2025
615a34b
Revert "Move `empack_config.yaml` to inside the project"
agriyakhetarpal Jan 24, 2025
7b318ff
Revert "Set `DEFAULT_CONFIG_PATH` to `user_config_dir`"
agriyakhetarpal Jan 24, 2025
76e9405
Port `_do_i_own` helper method
agriyakhetarpal Jan 24, 2025
273966b
Check for both conda/virtualenv style venvs
agriyakhetarpal Jan 24, 2025
77f55e3
Fix type annotation
agriyakhetarpal Jan 24, 2025
59cde9c
Add docstring
agriyakhetarpal Jan 24, 2025
f8b075f
Fix linter error
agriyakhetarpal Jan 27, 2025
ed052c8
Install appdirs for tests
agriyakhetarpal Jan 27, 2025
3b3a32e
Fix linter error again
agriyakhetarpal Jan 27, 2025
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
0 config/empack_config.yaml → empack/empack_config.yaml
View file
Open in desktop
File renamed without changes.
5 changes: 3 additions & 2 deletions empack/pack.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from tempfile import TemporaryDirectory
from typing import Callable, Optional

from appdirs import user_cache_dir
from platformdirs import user_cache_dir

from .filter_env import filter_env, filter_pkg, iterate_env_pkg_meta
from .micromamba_wrapper import create_environment
Expand All @@ -16,7 +16,8 @@
EMPACK_CACHE_DIR = Path(user_cache_dir("empack"))
PACKED_PACKAGES_CACHE_DIR = EMPACK_CACHE_DIR / "packed_packages_cache"
PACKED_PACKAGES_CACHE_DIR.mkdir(parents=True, exist_ok=True)
DEFAULT_CONFIG_PATH = Path(sys.prefix) / "share" / "empack" / "empack_config.yaml"
# Ensure that the config is tied to a particular installation of empack
DEFAULT_CONFIG_PATH = Path(__file__).parent / "empack_config.yaml"


def filename_base_from_meta(pkg_meta):
Expand Down
5 changes: 1 addition & 4 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ classifiers = [
"Programming Language :: Python :: 3.11",
]
dependencies = [
"appdirs",
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems the CI says appdirs is still being used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be addressed by emscripten-forge/pyjs-code-runner#15, marking for completeness :)

"platformdirs",
"networkx",
"pyyaml",
"requests",
Expand All @@ -48,9 +48,6 @@ empack = "empack.cli.main:app"
[project.urls]
Homepage = "https://github.com/emscripten-forge/empack"

[tool.hatch.build.targets.wheel.shared-data]
config = "share/empack"
Copy link
Contributor

Choose a reason for hiding this comment

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

I hope I won't be too annoying to you on this. I have a preference for this to stay 🫤

I believe we should make sure we can find this path when using pyenv.

It seems jupyter has something like that https://github.com/jupyter/jupyter_core/blob/main/jupyter_core/paths.py#L102 for finding whether or not it should use user-level paths or conda environment paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope I won't be too annoying to you on this.

Absolutely not! :D I'd also love to ensure pyenv works and not move anything else. Let me port Jupyter's thing here, then.


[tool.hatch.build.targets.sdist]
exclude = [
".github",
Expand Down
4 changes: 2 additions & 2 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
import pytest

from empack.file_patterns import pkg_file_filter_from_yaml
from empack.pack import DEFAULT_CONFIG_PATH

THIS_DIR = os.path.dirname(os.path.realpath(__file__))
CONFIG_PATH = os.path.join(THIS_DIR, "..", "config", "empack_config.yaml")
CONFIG_PATH = DEFAULT_CONFIG_PATH
FILE_FILTERS = pkg_file_filter_from_yaml(CONFIG_PATH)
CHANNELS = ["conda-forge", "https://repo.mamba.pm/emscripten-forge"]

Expand Down