Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

fix cli test #12

Merged
merged 3 commits into from
Nov 9, 2023
Merged

fix cli test #12

merged 3 commits into from
Nov 9, 2023

Conversation

PeterKraus
Copy link
Collaborator

@PeterKraus PeterKraus commented Nov 9, 2023

Fixes the API on my windows machine, also fixes CLI-with-venv test.

@PeterKraus
Copy link
Collaborator Author

@ml-evs This one is a PR-to-your-PR, feel free to merge when you have time.

Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Super helpful, thanks @PeterKraus

py_cmd = "import subprocess; return subprocess.check_output(f'{command}', shell=True)"
command = [str(self.venv_dir / "bin" / "python"), "-c", py_cmd]
print(f"Executing {command=} in venv")
py_cmd = f"import subprocess; subprocess.check_output(r'{command}', shell=True)"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
py_cmd = f"import subprocess; subprocess.check_output(r'{command}', shell=True)"
py_cmd = f"import subprocess; subprocess.check_output(r'{command}', shell=True)"

Haha thanks, I wrote this as my Eurostar started boarding and then wasted the entire journey just re-running the tests without making any changes and scratching my head...

@ml-evs ml-evs merged commit 99d53d3 into ml-evs/use_venvs Nov 9, 2023
3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants