Skip to content
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.

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

Support windows with juv run #54

Merged
merged 5 commits into from
Dec 4, 2024
Merged

Support windows with juv run #54

merged 5 commits into from
Dec 4, 2024

Conversation

ATL2001
Copy link
Contributor

@ATL2001 ATL2001 commented Nov 29, 2024

I only had to make a few code tweaks to get the code working on my windows box for firing up the subprocess, and I added raw string syntax for the notebook path in case the notebook's path has a \t in it so it doesnt change the \t into a tab. or a \u into a unicode escape etc. and I added ignore_cleanup_errors=True to all the tempdirectory usage because without it, an error would throw anytime I shutdown jupyterlab because the nbsignatures.db of jupyter was locked.

To get the tests to pass I had to replace the pipes at the end of some of the lines with the pipe symbol from my keyboard (I'm not going to pretend to be an encoding expert, but when I did a find/replace on the pipe symbols in the existing code with the pipe symbol on my keyboard, magically the tests passed for me)

let me know what you think!

…rlab, and added raw string identifier before the notebook path in case the notebook's path has a \t in it, so that doesnt get turned into a tab
@manzt manzt added the enhancement New feature or request label Dec 3, 2024
Copy link
Owner

@manzt manzt left a comment

Choose a reason for hiding this comment

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

Thanks for the awesome contribution! Sorry it took me so long for the review-preparing for my phd defense next week.

Just a couple of comments that hopefully we can get addressed quickly! Thanks again.

PS: Also when finished, you can run uv run ruff format to format the files so the linting checks pass as well.

src/juv/_run_replace.py Outdated Show resolved Hide resolved
src/juv/_run_template.py Outdated Show resolved Hide resolved
src/juv/_run_template.py Show resolved Hide resolved
tests/test_juv.py Show resolved Hide resolved
@ATL2001
Copy link
Contributor Author

ATL2001 commented Dec 4, 2024

I thought I had run ruff format before committing stuff, but I'm sure I did this time! good luck on the defense! 👍

Copy link
Owner

@manzt manzt left a comment

Choose a reason for hiding this comment

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

Thanks for the awesome contribution!

Copy link
Owner

@manzt manzt left a comment

Choose a reason for hiding this comment

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

Thanks for the awesome contribution!

@manzt manzt changed the title Add windows support to juv Support windows with juv run Dec 4, 2024
@manzt manzt merged commit 18f6263 into manzt:main Dec 4, 2024
13 checks passed
@ATL2001
Copy link
Contributor Author

ATL2001 commented Dec 4, 2024

happy to help! thanks for making it in the first place! I can't wait to show it off to some people, do you have any idea when the windows stuff will be available from pypi?

  • Aaron

@manzt
Copy link
Owner

manzt commented Dec 4, 2024

Shipped in v0.2.26! Thanks

@manzt
Copy link
Owner

manzt commented Dec 4, 2024

Whoops, there was a bug where TemporaryDirectory in the templated didn't get updated. Fixed in v0.2.27.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants