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/avoid redundant function calls #5280

Merged
merged 5 commits into from
Apr 17, 2024
Merged

Conversation

kenodegard
Copy link
Contributor

@kenodegard kenodegard commented Apr 12, 2024

Description

Drive-by improvements:

  • use set operations instead of for-loop + if-clause
  • use argument unpacking instead of set.update

Split from:

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?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Apr 12, 2024
Copy link

codspeed-hq bot commented Apr 12, 2024

CodSpeed Performance Report

Merging #5280 will not alter performance

Comparing kenodegard:cleanup (210c30a) with main (51793ce)

Summary

✅ 3 untouched benchmarks

@kenodegard kenodegard marked this pull request as ready for review April 12, 2024 20:14
@kenodegard kenodegard requested a review from a team as a code owner April 12, 2024 20:14
beckermr
beckermr previously approved these changes Apr 13, 2024
Copy link
Contributor

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

LGTM!

beeankha
beeankha previously approved these changes Apr 15, 2024
Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Nice improvement Ken! 😀

Had a question on whether we can apply a similar strategy to some other code here

Comment on lines 707 to 712
loop_vars = [
k
for k in variants[0]
for k, v in first_variant.items()
if k not in special_keys
and (
not loop_only
or any(variant[k] != variants[0][k] for variant in variants[1:])
)
and (not loop_only or any(variant[k] != v for variant in other_variants))
]
Copy link
Member

Choose a reason for hiding this comment

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

Could this also be written in terms of sets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kenodegard kenodegard dismissed stale reviews from beeankha and beckermr via 210c30a April 17, 2024 04:19
Comment on lines +704 to +705
@deprecated.argument("24.5", "24.7", "loop_only")
def get_vars(variants: Iterable[dict[str, Any]]) -> set[str]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opted to deprecate loop_only since get_vars is only invoked from one place and for that call loop_only=True (see conda_build.metadata.MetaData.get_loop_vars).

@kenodegard kenodegard merged commit 2d3270d into conda:main Apr 17, 2024
28 checks passed
@kenodegard kenodegard deleted the cleanup branch April 17, 2024 21:41
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.

7 participants