Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fixed imports and docstring #95

Merged
merged 2 commits into from
Sep 19, 2024
Merged

fixed imports and docstring #95

merged 2 commits into from
Sep 19, 2024

Conversation

d-kleine
Copy link
Contributor

  • removed duplicated import of "platform" pkg
  • fixed pip install command in docstring

@d-kleine d-kleine marked this pull request as ready for review September 19, 2024 06:06
@@ -340,7 +339,7 @@ def _get_conda_env():
def _get_gpu_info():
if py3nvml is None:
return {"GPU Info": "Install the gpu extra "
"(pip install 'watermark[gpu]') "
"(pip install watermark[gpu]) "
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the PR! I think this may not work on certain terminal shells (bash I think) and require the comment characters. Are the comment characters cause any issues on your terminal, which is why you removed them, or was this more of a cosmetic change?

Copy link
Contributor Author

@d-kleine d-kleine Sep 19, 2024

Choose a reason for hiding this comment

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

Are the comment characters cause any issues on your terminal, which is why you removed them, or was this more of a cosmetic change?

Did/Does not work for me in command shell, only when removing the ' characters:
grafik

grafik

Edit: Seems to be a command shell issue on Windows. It works fine in WSL (Ubuntu) and in Powershell on Windows.

Copy link
Owner

Choose a reason for hiding this comment

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

Huh, wow. What happens if you use the double-quote character: pip install "watermark[gpu]"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, looks better 🙂
grafik

Copy link
Owner

Choose a reason for hiding this comment

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

Oh nice. Just updated the line in this PR accordingly. Would that work for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lgtm!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rasbt Ah, there is small typo, there is a wrong ) after [gpu]. It should be
'(pip install "watermark[gpu]") '
not
'(pip install "watermark[gpu])") '

watermark/watermark.py Outdated Show resolved Hide resolved
@rasbt rasbt merged commit cf5517b into rasbt:master Sep 19, 2024
1 check passed
@d-kleine d-kleine mentioned this pull request Sep 21, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants