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

Deprecate conda.models.dist and conda.plan #13557

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mbargull
Copy link
Member

@mbargull mbargull commented Feb 2, 2024

Description

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?

Following the removal of conda.models.dist and conda.plan's usage in conda-build via conda/conda-build#5074 , this moves the few functions of these modules that are used within conda itself to other modules and marks both modules as pending for deprecation.

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Feb 2, 2024
@mbargull mbargull force-pushed the deprecate-dist-and-plan branch 2 times, most recently from 1cd92b0 to 733e081 Compare February 2, 2024 09:43
Copy link

codspeed-hq bot commented Feb 2, 2024

CodSpeed Performance Report

Merging #13557 will create unknown performance changes

Comparing mbargull:deprecate-dist-and-plan (897a906) with main (ddfe118)

Summary

⚠️ No benchmarks were detected in both the base of the PR and the PR.

@@ -1614,3 +1622,61 @@ def messages(prefix):
return m
finally:
rm_rf(path)


def _get_best_prec_match(precs):
Copy link
Member Author

Choose a reason for hiding this comment

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

We may want to change the name to not carry the underscore prefix since it's used in an other module (conda.misc also imported .plan._get_best_prec_match before).

from .channel import Channel
from .package_info import PackageInfo
from .records import PackageRecord

log = getLogger(__name__)

deprecated.module(
"24.3",
"24.9",
Copy link
Member Author

@mbargull mbargull Feb 2, 2024

Choose a reason for hiding this comment

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

This is the earliest I'd be comfortable with.
Dist is used elsewhere outside of https://github.com/conda .
Unless it induces noticeable maintenance burden, we can also postpone the actual removal to early 2025 or so.
Marking this for 24.9 just gives us the possibility to remove it earlier.

deprecated.module(
"24.3",
"24.9",
addendum="Use `conda.models.records.PackageRecord` instead.",
Copy link
Member Author

Choose a reason for hiding this comment

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

PackageRecord of course does not cover every functionality of this module one to one.
Feel free to add additional deprecation notes to symbols of this module as you see fit!

@@ -519,7 +519,7 @@ environment..." step in the CLI report.
[conda.models.match_spec:MatchSpec]: https://github.com/conda/conda/blob/4.11.0/conda/models/match_spec.py#L73
[conda.misc:explicit]: https://github.com/conda/conda/blob/4.11.0/conda/misc.py#L52
[conda.misc:clone_env]:https://github.com/conda/conda/blob/4.11.0/conda/misc.py#L187
[conda.plan:revert_actions]: https://github.com/conda/conda/blob/4.11.0/conda/plan.py#L279
[conda.core.link:revert_actions]: https://github.com/conda/conda/blob/24.1.0/conda/plan.py#L298
Copy link
Member Author

Choose a reason for hiding this comment

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

If/when we merge this PR, we should open a tracking issue to update this link to actually point to the correct module from a new release.

Comment on lines -26 to -36
from .core.index import LAST_CHANNEL_URLS, _supplement_index_with_prefix
from .core.link import PrefixSetup, UnlinkLinkTransaction
from .core.solve import diff_for_unlink_link_precs
from .exceptions import CondaIndexError, PackagesNotFoundError
from .history import History
from .core.index import LAST_CHANNEL_URLS
from .core.link import _get_best_prec_match, revert_actions # noqa: F401
from .deprecations import deprecated
from .instructions import FETCH, LINK, SYMLINK_CONDA, UNLINK
from .models.channel import Channel, prioritize_channels
from .models.dist import Dist
from .models.enums import LinkType
from .models.match_spec import ChannelMatch
from .models.prefix_graph import PrefixGraph
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we add deprecated.constant(...)s for all re-exported symbols this removes?

@mbargull mbargull marked this pull request as ready for review February 2, 2024 10:10
@mbargull mbargull requested a review from a team as a code owner February 2, 2024 10:10
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.

2 participants