-
Notifications
You must be signed in to change notification settings - Fork 30
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
Align path format for windows. #243
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @specter119!
I'm wondering if we should use as_posix()
only when passing paths to the front-end, and not use it or forward slashes (/
) in the back-end.
plugins/auth/fps_auth/db.py
Outdated
@@ -18,7 +18,7 @@ | |||
|
|||
auth_config = get_config(AuthConfig) | |||
|
|||
jupyter_dir = Path.home() / ".local" / "share" / "jupyter" | |||
jupyter_dir = Path.home() / ".local/share/jupyter" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the hard-coded /
going to play well on Windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, hard-code /
, works as excepted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can read, I think it works because Python does some tricks to convert forward slashes to backslashes if needed when opening files, but the best practice is still to use pathlib
's /
operator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert these changes.
We currently don't handle a root directory (see #236), but we definitely need this. |
I add these changes to my local jupyverse, and it works as excepted. Together, I will actively use jupyverse for a perioid of time. Since the behavior of jupyter-server (maybe related with tornado) break my daily work, and downgrade to tornado 6.1 doesn't help. |
Yes, jupyter-server's dependency on Tornado was one of the reasons to start jupyverse. |
This reverts commit edbba21.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Kyle, just a minor change.
This PR makes the following modifications.
overall:
str(pathlib.Path)
topathlib.Path.as_posix()
convertpathlib.Path / "a" / "b" / "c"
to pathlib.Path / "a/b/c"parent.parent
withparents[1]
for
plugins/lab/fps_lab/utils.py
:package
With these modifications, the following issues on the windows platform solved:
/ a\b\c /
, and can't enter the parent folder).package.json
without "jupyterlab"however, the following errors still exist, and I'm not sure they're windows specific:
--root-path
doesn't change the root folder, and the root folder is still.
pwd
is always the root folder and doesn't change following the editing file.To sum up, the unix like path is more friendly even in the windows platform, but str(pahtlib.Path) will convert the path as the native format.