Skip to content

Conversation

kenodegard
Copy link
Contributor

Description

Followup to #13880

Stop using deprecated conda.core.index.get_index and conda.core.index.get_reduced_index.

Extracted from #12771

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@kenodegard kenodegard changed the title Replace get_index and get_reduced_index Replace get_index and get_reduced_index usage Jun 4, 2025
@github-project-automation github-project-automation bot moved this to 🆕 New in 🔎 Review Jun 4, 2025
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jun 4, 2025
Copy link

codspeed-hq bot commented Jun 4, 2025

CodSpeed Performance Report

Merging #14905 will not alter performance

Comparing kenodegard:cleanup-index-deprecation (6fa85a3) with main (998c033)

Summary

✅ 21 untouched benchmarks

@kenodegard
Copy link
Contributor Author

Blocked by #14924

@kenodegard kenodegard force-pushed the cleanup-index-deprecation branch from aad7a47 to 6fa85a3 Compare June 10, 2025 13:27
@kenodegard kenodegard moved this from 🆕 New to 🏗️ In Progress in 🔎 Review Jun 10, 2025
@kenodegard kenodegard marked this pull request as ready for review June 10, 2025 14:13
@kenodegard kenodegard requested a review from a team as a code owner June 10, 2025 14:13
@kenodegard kenodegard moved this from 🏗️ In Progress to 👀 In Review in 🔎 Review Jun 10, 2025
@@ -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]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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:
Copy link
Contributor

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?

Copy link
Contributor Author

@kenodegard kenodegard Jun 10, 2025

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 🤷🏼‍♂️

@jezdez jezdez self-requested a review June 23, 2025 13:33
@jezdez jezdez added this to the 25.7.x milestone Jun 23, 2025
Copy link
Contributor

@travishathaway travishathaway left a 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.

@github-project-automation github-project-automation bot moved this from 👀 In Review to ✅ Approved in 🔎 Review Jun 23, 2025
jaimergp

This comment was marked as duplicate.

Copy link
Contributor

@jaimergp jaimergp left a 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.

@kenodegard kenodegard merged commit edd29aa into conda:main Jun 23, 2025
75 checks passed
@kenodegard kenodegard deleted the cleanup-index-deprecation branch June 23, 2025 16:02
@github-project-automation github-project-automation bot moved this from ✅ Approved to 🏁 Done in 🔎 Review Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants