Skip to content

Slightly improve performance in engines/utils.py#8747

Merged
ericspod merged 5 commits intoProject-MONAI:devfrom
benediktjohannes:patch-4
Apr 4, 2026
Merged

Slightly improve performance in engines/utils.py#8747
ericspod merged 5 commits intoProject-MONAI:devfrom
benediktjohannes:patch-4

Conversation

@benediktjohannes
Copy link
Copy Markdown
Contributor

Description

This change probably improves performance slightly (unforetunately, I don't have a local instance of MONAI installed in order to test this, but I'm pretty sure from looking at the code that using [], {} and kwargs_[k] = get_data(v) should be semantically the same and probably slightly better in performance, see also #1423 (credits to @de-sh for the nice fix 👍 ) where at least [] and {} were changed as a refactoring and kwargs[k] = _get_data(v) just seems better to me becuase I think that this saves some time due to dictionary lookups and temporary objects, but I'm not quite sure because it wasn't tested, so please correct me if I'm mistaken).

A few sentences describing the changes proposed in this pull request.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

Signed-off-by: Benedikt Johannes <benedikt.johannes.hofer@gmail.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a03c6105-60d8-400e-9630-fc56d787b3b3

📥 Commits

Reviewing files that changed from the base of the PR and between 540b662 and 94a465b.

📒 Files selected for processing (1)
  • monai/engines/utils.py
✅ Files skipped from review due to trivial changes (1)
  • monai/engines/utils.py

📝 Walkthrough

Walkthrough

Updated container initializations in monai/engines/utils.py within PrepareBatchExtraInput.__call__: replaced list() with [] and dict() with {}. The subsequent population logic (args_.append(...), kwargs_.update(...)/keyword extraction) and return values (tuple(args_), kwargs_) remain unchanged; no control-flow or error-handling changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title directly matches the main change: replacing list()/dict() calls with []/{}. Concise and specific.
Description check ✅ Passed Description covers the change purpose and includes checklist. However, it lacks technical clarity about what specific optimization was attempted beyond general claims.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

ericspod added a commit that referenced this pull request Feb 27, 2026
…r.py (#8749)

### Description
See also #8747 and #8748 

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing
functionality to change).
- [ ] New tests added to cover the changes.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [ ] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [ ] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

---------

Signed-off-by: Benedikt Johannes <benedikt.johannes.hofer@gmail.com>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Signed-off-by: Benedikt Johannes <benedikt.johannes.hofer@gmail.com>
@benediktjohannes
Copy link
Copy Markdown
Contributor Author

@ericspod could you maybe open an extra PR for your additional suggestions and merge this for now, so that we can check on the more complex (and test requiring) changes lateron, but already have these safe slight improvements? Thanks!

ericspod added a commit that referenced this pull request Apr 4, 2026
### Description
See also #8747 

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing
functionality to change).
- [ ] New tests added to cover the changes.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [ ] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [ ] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: Benedikt Johannes <benedikt.johannes.hofer@gmail.com>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Copy link
Copy Markdown
Member

@ericspod ericspod left a comment

Choose a reason for hiding this comment

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

We can merge this now then if you want to come back to the suggestion later.

@ericspod ericspod enabled auto-merge (squash) April 4, 2026 22:07
@ericspod ericspod merged commit a65689e into Project-MONAI:dev Apr 4, 2026
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants