-
Notifications
You must be signed in to change notification settings - Fork 426
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
Conversation
CodSpeed Performance ReportMerging #5333 will not alter performanceComparing Summary
|
2001bc7
to
3891651
Compare
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.
Confirmed this contains all of the removals, now that it's been double checked can we sort them alphabetically?
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.
Sounds good. I'll also try to make the news file easier to read like @jakirkham suggested
conda_build/index.py
Outdated
_channel_data = {} | ||
deprecated.constant("24.1", "24.7", "channel_data", _channel_data) |
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.
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?
news/5333-remove-24.7.x-deprecations
Outdated
### 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) |
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.
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`
* ...
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.
That's a great idea!
… more generally readable
news/5333-remove-24.7.x-deprecations
Outdated
### 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)` |
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.
What do you think of this layout change/update, @kenodegard and @jakirkham ?
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.
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.
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.
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
conda_build/index.py
Outdated
mtime = 0 | ||
|
||
_channel_data = {} |
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.
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.
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
- remove everything from
expanded_channels = ...
(currently line 132) until_channel_data["defaults"] = ...
(currently line 175), and - 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(...)
.
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.
oooh! that makes much more sense!
yeah I'd just return an empty dict then so we don't modify the return structure/type
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.
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.
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.
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.
news/5333-remove-24.7.x-deprecations
Outdated
* `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` |
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.
* `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?
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.
Oh good call, I'll update the news file with this info!
The _channel_data
variable issue has been removed/taken care of
Description
This PR removes everything marked as deprecated in conda-build
24.7.x
Checklist - did you ...
news
directory (using the template) for the next release's release notes?Add / update necessary tests?Add / update outdated documentation?