-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Replace get_index
and get_reduced_index
usage
#14905
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
Conversation
get_index
and get_reduced_index
get_index
and get_reduced_index
usage
CodSpeed Performance ReportMerging #14905 will not alter performanceComparing Summary
|
Blocked by #14924 |
aad7a47
to
6fa85a3
Compare
@@ -1261,7 +1263,7 @@ def _notify_conda_outdated(self, link_precs): | |||
file=sys.stderr, | |||
) | |||
|
|||
def _prepare(self, prepared_specs): | |||
def _prepare(self, prepared_specs) -> tuple[ReducedIndex, Resolve]: |
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.
def _prepare(self, prepared_specs) -> tuple[ReducedIndex, Resolve]: | |
def _prepare(self, prepared_specs: Iterable[MatchSpec]) -> tuple[ReducedIndex, Resolve]: |
I think? Double check though.
def test_supplement_index_with_system(): | ||
index = {} | ||
_supplement_index_with_system(index) | ||
def test_deprecated_supplement_index_with_system() -> None: |
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.
Why are we adding return annotations to tests? 🤔 They never return anything, do they?
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.
At one point mypy
wouldn't by default type check functions unless either the args or return was annotated, so I added these None
returns to enable the type checking in my IDE for functions without any args, I've just gotten in the habit of consistently adding them 🤷🏼♂️
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.
I think this is ready to merge. Let's wait for the other assigned reviewers to chime in too.
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.
Leaving aside I just realized that some tests are annotated with -> None
return types, and that that's not within the scope of this PR (we should address that conversation separately), the changes proposed here look ok to me.
Description
Followup to #13880
Stop using deprecated
conda.core.index.get_index
andconda.core.index.get_reduced_index
.Extracted from #12771
Checklist - did you ...
Add a file to thenews
directory (using the template) for the next release's release notes?Add / update outdated documentation?