-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add a lazy is_sorted function to collections #159
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #159 +/- ##
==========================================
+ Coverage 88.38% 88.44% +0.06%
==========================================
Files 16 16
Lines 1773 1791 +18
Branches 377 381 +4
==========================================
+ Hits 1567 1584 +17
Misses 136 136
- Partials 70 71 +1 ☔ View full report in Codecov by Sentry. |
fgpyo/collections/__init__.py
Outdated
IterType = TypeVar("IterType") | ||
|
||
_LessThanOrEqualType = TypeVar("_LessThanOrEqualType", bound=_SupportsLessThanOrEqual) |
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 it appropriate for this type be "private" when it is used as a type parameter in a "public" method below?
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.
Can do!
I notice fgpyo doesn't use __init__.py
for public namespace definition which makes defining a public API trickier. For example, the function pairwise
in this PR is now exposed as our public API instead of just as an import or a helper. I do see Matt is tracking that issue here, though:
EDIT: I renamed pairwise
to _pairwise
to avoid this without the need to restructure the project.
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.
Agreed, one way to manage what is public versus private is #136, but another way to solve it is to the way we're doing here. Moving things out of __init__.py
into submodules doesn't solve this, since the methods/classes can be public there, but it does help with namespace issues. Lets move that discussion to #136 since the namespace issue is compelling, and any new method can always be private like we have here for pairwise.
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.
lgtm
- pymdownx.highlight: | ||
anchor_linenums: true | ||
line_spans: __span | ||
pygments_lang_class: true | ||
- pymdownx.inlinehilite | ||
- pymdownx.snippets | ||
- pymdownx.superfences |
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.
No description provided.