Skip to content

Replace IndexedSet with Sequence / tuple; additional typing tweaks - #879

Draft
dholth wants to merge 2 commits into
mainfrom
drop-indexedset
Draft

Replace IndexedSet with Sequence / tuple; additional typing tweaks#879
dholth wants to merge 2 commits into
mainfrom
drop-indexedset

Conversation

@dholth

@dholth dholth commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

Description

See conda/conda#15742

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?

@dholth
dholth requested a review from jaimergp February 27, 2026 17:46
@github-project-automation github-project-automation Bot moved this to 🆕 New in 🔎 Review Feb 27, 2026
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Feb 27, 2026
force_remove: bool | _Null = NULL,
should_retry_solve: bool = False,
) -> IndexedSet[PackageRecord]:
) -> Sequence[PackageRecord]:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Base class only says (in an inaccurate docstring) that this returns a tuple of PackageRef

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can go with the more accurate:

Suggested change
) -> Sequence[PackageRecord]:
) -> tuple[PackageRecord, ...]:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

tuple is more concrete, but Sequence is accurate and has a nicer "all elements of the same type" annotation without ...


@property
def current_solution(self) -> IndexedSet[PackageRecord]:
def current_solution(self) -> Sequence[PackageRecord]:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
def current_solution(self) -> Sequence[PackageRecord]:
def current_solution(self) -> tuple[PackageRecord, ...]:

return {name: spec for name, spec in self.specs.items() if name.startswith("__")}

def early_exit(self) -> IndexedSet[PackageRecord] | None:
def early_exit(self) -> Sequence[PackageRecord] | None:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
def early_exit(self) -> Sequence[PackageRecord] | None:
def early_exit(self) -> tuple[PackageRecord, ...] | None:

@dholth dholth Feb 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I usually put the more generic type in annotations. Sequence[] would allow IndexedSet or tuple or list for example. Especially in the base class in conda.

@dholth
dholth force-pushed the drop-indexedset branch from 2280d7a to 80297ca Compare June 16, 2026 18:07
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

Status: 🆕 New

Development

Successfully merging this pull request may close these issues.

3 participants