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

Remove deprecated code for conda-build 24.7 #5333

Merged
merged 18 commits into from
May 29, 2024

Conversation

beeankha
Copy link
Contributor

@beeankha beeankha commented May 10, 2024

Description

This PR removes everything marked as deprecated in conda-build 24.7.x

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?

@beeankha beeankha added source::anaconda created by members of Anaconda, Inc. type::deprecation requests removal of deprecated feature(s) labels May 10, 2024
@beeankha beeankha self-assigned this May 10, 2024
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label May 10, 2024
Copy link

codspeed-hq bot commented May 10, 2024

CodSpeed Performance Report

Merging #5333 will not alter performance

Comparing beeankha:remove-24.7.x-deprecations (35170f9) with main (d9edc57)

Summary

✅ 3 untouched benchmarks

@beeankha beeankha force-pushed the remove-24.7.x-deprecations branch from 2001bc7 to 3891651 Compare May 12, 2024 15:11
@beeankha beeankha changed the title [WIP] Remove deprecated code for conda-build 24.7 Remove deprecated code for conda-build 24.7 May 13, 2024
@beeankha beeankha marked this pull request as ready for review May 13, 2024 21:27
@beeankha beeankha requested a review from a team as a code owner May 13, 2024 21:27
Copy link
Contributor

Choose a reason for hiding this comment

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

Confirmed this contains all of the removals, now that it's been double checked can we sort them alphabetically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I'll also try to make the news file easier to read like @jakirkham suggested

Comment on lines 30 to 32
_channel_data = {}
deprecated.constant("24.1", "24.7", "channel_data", _channel_data)
Copy link
Contributor

Choose a reason for hiding this comment

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

channel_data was previously postponed since the global variable (_channel_data) was still in use. What would it take for us to remove this global variable and instead only use a local variable?

Comment on lines 9 to 19
### Deprecations

* Remove `conda_build.conda_interface.Completer`. (#5333)
* Remove `conda_build.conda_interface.CondaSession`. Use `conda.gateways.connection.session.CondaSession` instead. (#5333)
* Remove `conda_build.conda_interface.InstalledPackages`. (#5333)
* Remove `conda_build.conda_interface.NoPackagesFound`. Use `conda.exceptions.ResolvePackageNotFound` instead. (#5333)
* Remove `conda_build.conda_interface.Unsatisfiable`. Use `conda.exceptions.UnsatisfiableError` instead. (#5333)
* Remove `conda_build.conda_interface.symlink_conda`. (#5333)
* Remove `conda_build.conda_interface.ArgumentParser`. Use `conda.cli.conda_argparse.ArgumentParser` instead. (#5333)
* Remove `conda_build.conda_interface.add_parser_channels`. Use `conda.cli.helpers.add_parser_channels` instead. (#5333)
* Remove `conda_build.conda_interface.add_parser_prefix`. Use `conda.cli.helpers.add_parser_prefix` instead. (#5333)
Copy link
Member

Choose a reason for hiding this comment

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

Should we link out to a doc, summarize or streamline the list a bit?

An example of streamlining:

### Deprecations

* Remove `conda_build.conda_interface.*` component. (#5333):
  * `Completer`
  * ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great idea!

Comment on lines 9 to 87
### Deprecations

* Remove the following deprecations (#5333):
* `conda_build.config.Config.override_channels` (use `conda.base.context.context.channels` instead)
* `conda_build.config.noarch_python_build_age_default`
* `conda_build.conda_interface.add_parser_channels` (use `conda.cli.helpers.add_parser_channels` instead)
* `conda_build.conda_interface.add_parser_prefix` (use `conda.cli.helpers.add_parser_prefix` instead)
* `conda_build.conda_interface.ArgumentParser` (use `conda.cli.conda_argparse.ArgumentParser` instead)
* `conda_build.conda_interface.binstar_upload` (use `conda.base.context.context.binstar_upload` instead)
* `conda_build.conda_interface.cc_conda_build` (use `conda.base.context.context.conda_build` instead)
* `conda_build.conda_interface.cc_platform` (use `conda.base.context.context.platform` instead)
* `conda_build.conda_interface.Channel` (use `conda.models.channel.Channel` instead)
* `conda_build.conda_interface.Completer`
* `conda_build.conda_interface.configparser` (use `configparser` instead)
* `conda_build.conda_interface.CondaError` (use `conda.exceptions.CondaError` instead)
* `conda_build.conda_interface.CondaHTTPError` (use `conda.exceptions.CondaHTTPError` instead)
* `conda_build.conda_interface.CondaSession` (use `conda.gateways.connection.session.CondaSession` instead)
* `conda_build.conda_interface.CONDA_VERSION` (use `conda.__version__` instead)
* `conda_build.conda_interface.context` (use `conda.base.context.context` instead)
* `conda_build.conda_interface.create_default_packages` (use `conda.base.context.context.create_default_packages` instead)
* `conda_build.conda_interface.default_python` (use `conda.base.context.context.default_python` instead)
* `conda_build.conda_interface.determine_target_prefix` (use `conda.base.context.determine_target_prefix` instead)
* `conda_build.conda_interface.download` (use `conda.gateways.connection.download.download` instead)
* `conda_build.conda_interface.env_path_backup_var_exists`
* `conda_build.conda_interface.envs_dirs` (use `conda.base.context.context.envs_dirs` instead)
* `conda_build.conda_interface.EntityEncoder` (use `conda.auxlib.entity.EntityEncoder` instead)
* `conda_build.conda_interface.FileMode` (use `conda.models.enums.FileMode` instead)
* `conda_build.conda_interface.get_conda_build_local_url` (use `conda.models.channel.get_conda_build_local_url` instead)
* `conda_build.conda_interface.get_conda_channel` (use `conda.models.channel.Channel.from_value` instead)
* `conda_build.conda_interface.get_prefix` (use `conda.base.context.context.target_prefix` instead)
* `conda_build.conda_interface.get_rc_urls` (use `conda.base.context.context.channels` instead)
* `conda_build.conda_interface.human_bytes` (use `conda.utils.human_bytes` instead)
* `conda_build.conda_interface.import_module` (use `importlib.import_module` instead)
* `conda_build.conda_interface.input` (use `input` instead)
* `conda_build.conda_interface.InstalledPackages`
* `conda_build.conda_interface.lchmod` (use `conda.gateways.disk.link.lchmod` instead)
* `conda_build.conda_interface.LinkError` (use `conda.exceptions.LinkError` instead)
* `conda_build.conda_interface.LockError` (use `conda.exceptions.LockError` instead)
* `conda_build.conda_interface.MatchSpec` (use `conda.models.match_spec.MatchSpec` instead)
* `conda_build.conda_interface.non_x86_linux_machines` (use `conda.base.context.non_x86_machines` instead)
* `conda_build.conda_interface.NoPackagesFound` (use `conda.exceptions.ResolvePackageNotFound` instead)
* `conda_build.conda_interface.NoPackagesFoundError` (use `conda.exceptions.NoPackagesFoundError` instead)
* `conda_build.conda_interface.normalized_version` (use `conda.models.version.normalized_version` instead)
* `conda_build.conda_interface.os` (use `os` instead)
* `conda_build.conda_interface.PackageRecord` (use `conda.models.records.PackageRecord` instead)
* `conda_build.conda_interface.PaddingError` (use `conda.exceptions.PaddingError` instead)
* `conda_build.conda_interface.partial` (use `functools.partial` instead)
* `conda_build.conda_interface.PathType` (use `conda.models.enums.PathType` instead)
* `conda_build.conda_interface.pkgs_dirs` (use `conda.base.context.context.pkgs_dirs` instead)
* `conda_build.conda_interface.prefix_placeholder` (use `conda.base.constants.PREFIX_PLACEHOLDER` instead)
* `conda_build.conda_interface.ProgressiveFetchExtract` (use `conda.core.package_cache_data.ProgressiveFetchExtract` instead)
* `conda_build.conda_interface.reset_context` (use `conda.base.context.reset_context` instead)
* `conda_build.conda_interface.Resolve` (use `conda.resolve.Resolve` instead)
* `conda_build.conda_interface.rm_rf` (use `conda_build.utils.rm_rf` instead)
* `conda_build.conda_interface.root_dir` (use `conda.base.context.context.root_prefix` instead)
* `conda_build.conda_interface.root_writable` (use `conda.base.context.context.root_writable` instead)
* `conda_build.conda_interface.spec_from_line` (use `conda.cli.common.spec_from_line` instead)
* `conda_build.conda_interface.specs_from_args` (use `conda.cli.common.specs_from_args` instead)
* `conda_build.conda_interface.specs_from_url` (use `conda.cli.common.specs_from_url` instead)
* `conda_build.conda_interface.StringIO` (use `io.StringIO` instead)
* `conda_build.conda_interface.subdir` (use `conda.base.context.context.subdir` instead)
* `conda_build.conda_interface.symlink_conda`
* `conda_build.conda_interface.TemporaryDirectory` (use `conda.gateways.disk.create.TemporaryDirectory` instead)
* `conda_build.conda_interface.TmpDownload` (use `conda.gateways.connection.download.TmpDownload` instead)
* `conda_build.conda_interface._toposort` (use `conda.common.toposort._toposort` instead)
* `conda_build.conda_interface.unix_path_to_win` (use `conda.utils.unix_path_to_win` instead)
* `conda_build.conda_interface.untracked` (use `conda.misc.untracked` instead)
* `conda_build.conda_interface.Unsatisfiable` (use `conda.exceptions.UnsatisfiableError` instead)
* `conda_build.conda_interface.UnsatisfiableError` (use `conda.exceptions.UnsatisfiableError` instead)
* `conda_build.conda_interface.url_path` (use `conda.utils.url_path` instead)
* `conda_build.conda_interface.VersionOrder` (use `conda.models.version.VersionOrder` instead)
* `conda_build.conda_interface.walk_prefix` (use `conda.misc.walk_prefix` instead)
* `conda_build.conda_interface.win_path_to_unix` (use `conda.common.path.win_path_to_unix` instead)
* `conda_build.index.channel_data`
* `conda_build.utils._convert_lists_to_sets` (use `frozendict.deepfreeze` instead)
* `conda_build.utils.HashableDict` (use `frozendict.deepfreeze` instead)
* `conda_build.utils.represent_hashabledict` (use `frozendict.deepfreeze` instead)
* `conda_build.utils.rm_rf(config)`
* `conda_build.variants.get_vars(loop_only)`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think of this layout change/update, @kenodegard and @jakirkham ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I don't see much of a difference, it's still hard to read but that's simply a product of a lot of deprecations. We could truncate all of the conda_interface deprecations into Removed `conda_build.conda_interface`. but we'd loose the more detailed information about what users should be doing instead.

Copy link
Member

Choose a reason for hiding this comment

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

This seems reasonable

Was starting to wonder if we should capture this sort of thing in a linter tool (like ruff or similar). That would help us apply these changes throughout the different Conda* codebases

Maybe we would also be less reliant on having this in the release notes

@beeankha beeankha marked this pull request as draft May 15, 2024 18:21
mtime = 0

_channel_data = {}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @mbargull ! Wanted to ping you directly per your work in #5152 marking the channel_data constant for deprecation; does this update to _channel_data (removing the global variable and making this a local variable instead) look OK to you?

Copy link
Member

@mbargull mbargull May 23, 2024

Choose a reason for hiding this comment

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

No quite -- meaning, making it local doesn't take it far enough ;).
Sorry, I should've added a code comment there:
The idea was to remove it altogether since we populate it, but don't actually use it as of now.
Meaning, we should

  1. remove everything from expanded_channels = ... (currently line 132) until _channel_data["defaults"] = ... (currently line 175), and
  2. either omit the third return value or just return None instead.

As for point 2: I did a GitHub code search for orgs conda, conda-forge, regro, conda-incubator, bioconda, mamba-org and could not find any usage of get_build_index outside of conda-build itself, so removing the return value is probably fine (returning None instead is a more defensive variant which wouldn't break uses of index, ts, _ = get_build_index(...).

Copy link
Contributor

Choose a reason for hiding this comment

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

oooh! that makes much more sense!

yeah I'd just return an empty dict then so we don't modify the return structure/type

Copy link
Member

Choose a reason for hiding this comment

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

This would, for one, of course remove unused code, but it would also avoid iterating over the index in expanded_channels = {rec.channel for rec in cached_index}.
On its own this doesn't yet change much really, since the index returned by conda.core.index.get_index/conda.core.index.fetch_index already has been iterated over (fetch_index does index.update((rec, rec) for rec in f.iter_records())).
But if/when we fix that initial iteration whilst addressing gh-5154, we can finally make use of deferred package record instantiation and shave off a good 30s and a whole lot of memory consumption.
Leaving the unused channel_data population in place would hinder that work, of course.

Copy link
Member

Choose a reason for hiding this comment

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

yeah I'd just return an empty dict then so we don't modify the return structure/type

I thought about doing this too -- but I think a None would be a bit better since it would at least intentionally break uses of the then invalid channel_data (i.e., avoid silent errors in case this is really used somewhere).
N.B.: All uses in conda-build itself just ignore that return value anyway.

@beeankha beeankha marked this pull request as ready for review May 22, 2024 17:12
* `conda_build.conda_interface.VersionOrder` (use `conda.models.version.VersionOrder` instead)
* `conda_build.conda_interface.walk_prefix` (use `conda.misc.walk_prefix` instead)
* `conda_build.conda_interface.win_path_to_unix` (use `conda.common.path.win_path_to_unix` instead)
* `conda_build.index.channel_data`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* `conda_build.index.channel_data`
* `conda_build.index.channel_data`
* `conda_build.index.get_build_index` return value for `channel_data` is now always `None`

or something along those lines maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh good call, I'll update the news file with this info!

mbargull
mbargull previously approved these changes May 29, 2024
@beeankha beeankha dismissed kenodegard’s stale review May 29, 2024 21:39

The _channel_data variable issue has been removed/taken care of

@beeankha beeankha merged commit d56495c into conda:main May 29, 2024
28 checks passed
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 source::anaconda created by members of Anaconda, Inc. type::deprecation requests removal of deprecated feature(s)
Projects
Status: 🏁 Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants