refactor: use pathlib #1948
refactor: use pathlib #1948
Conversation
AlbaHerrerias
force-pushed
the
nh-pathlib-refactor
branch
from
9871679 to
19e578d
Compare
|
Hey, we pushed some changes recently to conda-smithy that created merge conflicts. I'll get rid of them and then IMO this is good to go :) |
| # Install with powershell. | ||
| if os.path.exists(target_dir): | ||
| if Path(target_dir).exists(): | ||
| raise IOError("Installation directory already exists") | ||
| subprocess.check_call(cmd) | ||
|
|
||
| if not os.path.isdir(target_dir): | ||
| if not Path(target_dir).is_dir(): | ||
| raise RuntimeError("Failed to install miniconda :(") | ||
|
|
||
| if install_obvci: | ||
| conda_path = os.path.join(target_dir, bin_dir, "conda") | ||
| conda_path = Path(target_dir, bin_dir, "conda") | ||
| subprocess.check_call( |
There was a problem hiding this comment.
I don't understand why we are using several Path instantiations here when it's all target_dir. We should have something like:
target_dir = Path(target_dir)
if target_dir.exists():
...
if not target_dir.is_dir():
...
if install_obvci:
conda_path = target_dir / bin_dir / "conda"
and so on. I'm afraid this happens in more areas of the PR, so a careful review is advised imo.
| print("Updating " + fname) | ||
| if os.path.isfile(fname): | ||
| fname = Path(feedstock_directory, ".gitignore") | ||
| print("Updating ", fname.name) |
There was a problem hiding this comment.
| print("Updating ", fname.name) | |
| print("Updating", fname.name) |
There was a problem hiding this comment.
I am not sure, isn't .name going to give you only the file name (not the whole path as previously)?
I think it should be print("Updating", fname).
| "tokens", | ||
| project + ".json", | ||
| ) | ||
| token_file = str(Path(tmpdir, "tokens", f"{project}.json")) |
|
|
||
| # append the token if needed | ||
| if os.path.exists(token_file): | ||
| if Path(token_file).exists(): | ||
| with open(token_file, "r") as fp: |
There was a problem hiding this comment.
I think we can use token_file.open() (instead of open(token_file)
There was a problem hiding this comment.
or even json.loads(token_file.read_string())
There was a problem hiding this comment.
I think you mean .read_text(), right?
| if os.path.isfile(fname): | ||
| fname = Path(feedstock_directory, ".gitignore") | ||
| print("Updating ", fname.name) | ||
| if fname.is_file(): | ||
| with open(fname, "r") as f: |
There was a problem hiding this comment.
This can be written as fname.read_text() or fname.read_binary().
| @@ -41,7 +42,7 @@ def __init__( | |||
|
|
|||
| try: | |||
| with open( | |||
| os.path.expanduser("~/.conda-smithy/azure.token"), "r" | |||
There was a problem hiding this comment.
How about Path("~/.conda-smithy/azure.token").expanduser().read_text().strip()?
Hello 👋!
As part of the STF work for Conda, this PR refactors the usage of
osin favour ofpathlib, in an effort to modernise Conda.Thank you!
Checklist
newsentry