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

Py.typed #1685

Closed
wants to merge 3 commits into from
Closed

Py.typed #1685

wants to merge 3 commits into from

Conversation

sbrugman
Copy link
Contributor

@sbrugman sbrugman commented Feb 19, 2024

Suggested implementation for #1677

Summary

Add py.typed + cautionary docstring.
Importing these functions is discouraged, but not prohibited: users can do so at their own risk.

Included some best practices in the Python part, we should set the right example !

Atomic commits, see commit message for details.

Test Plan

- Explicit `check=False`
- Sort functions in their call order
- Use Pathlib for paths
return sys.prefix

return ""
from pathlib import Path
Copy link
Member

@zanieb zanieb Feb 19, 2024

Choose a reason for hiding this comment

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

We are explicitly not using pathlib because it's slower to import

Copy link
Member

Choose a reason for hiding this comment

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

Can you revert those changes?

strongly discouraged to import from our `uv` Python module, but you can do so at your
own risk.
"""
from uv.__main__ import detect_virtualenv, find_uv_bin
Copy link
Member

Choose a reason for hiding this comment

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

Let's not import then from __main__, let's do it the other way around (define them here and import them there).

I think detect_virtualenv should remain private as well.

Comment on lines +2 to +4
The Python API of `uv` is not guaranteed to be stable and may chance at any time. It is
strongly discouraged to import from our `uv` Python module, but you can do so at your
own risk.
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
The Python API of `uv` is not guaranteed to be stable and may chance at any time. It is
strongly discouraged to import from our `uv` Python module, but you can do so at your
own risk.
The Python API of `uv` is not guaranteed to be stable and may change at any time.

Fixed a typo and this seems sufficient. I was only strongly discouraging people from importing from __main__.

@zanieb zanieb added the enhancement New feature or improvement to existing functionality label Feb 19, 2024
@sbrugman
Copy link
Contributor Author

Superseded by #1728

@sbrugman sbrugman closed this Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants