Add hash_type default deprecation warnings to apps utils [pre-commit.ci autofix]

[Copilot]

Description

Follow-up to #9088 to restore a compatibility path for the hash_type default change in monai.apps.utils. This keeps the SHA-256 defaults in place while warning callers that still rely on the old implicit MD5 behavior.

• API deprecation warnings

  • Add deprecated_arg_default to check_hash, download_url, extractall, and download_and_extract.
  • Configure the warning for hash_type changing from "md5" to "sha256" when the argument is omitted.

• Docstrings

  • Add .. versionchanged:: 1.6.1 notes to the four public functions.
  • Document the MD5 → SHA-256 default change and the explicit hash_type="md5" opt-in for legacy callers.

• Coverage

  • Extend the existing apps hash tests to verify:
    • omitted hash_type emits a FutureWarning
    • explicit hash_type="sha256" does not warn
    • explicit hash_type="md5" continues to work

download_url(url, filepath, hash_val=expected_hash)          # warns on implicit old default change
download_url(url, filepath, hash_val=expected_hash, hash_type="sha256")  # no warning
download_url(url, filepath, hash_val=legacy_md5, hash_type="md5")        # explicit legacy behavior

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.

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: Project-MONAI/MONAI/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 65364daf-da04-4ab7-97a7-7f3001c955e7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[ericspod]

Hi @garciadias please have a look at this, it's something I neglected from the previous PR on hardening the hash checks.

[garciadias]

Using deprecated_arg_default here is the right call, but I think this particular "since" value makes the whole mechanism inert on exactly the builds that carry the breaking change.

deprecated_arg_default skips installation entirely when the running version is at or below "since" — see the is_not_yet_deprecated branch in monai/utils/deprecate_utils.py around line 283, which returns an identity decorator so no warning is ever emitted. The dev branch currently versions as 1.6.1rc0+N.g, which orders strictly before 1.6.1, so on every dev build and every 1.6.1 release candidate all four decorators in this file are no-ops.

Reproduced on a checkout of this branch:

monai.__version__ = 1.6.1rc0+6.g72a9d3b9
version_leq(v, "1.6.1") = True  and  v != "1.6.1"  ->  decorator inert
check_hash(filepath, sha256)  with hash_type omitted  ->  0 FutureWarnings

The same cause makes the new test fail locally on a checkout that has tags:

tests/apps/test_check_hash.py:105: AssertionError: 0 != 1
1 failed, 10 passed

CI stays green only because the unit-test jobs check out shallow without tags, so the version resolves to 0+untagged and deprecate_utils substitutes sys.maxsize, which activates the warning. Copying the same tree without .git gives 11 passed. So the green checks on this PR do not demonstrate that the warning works for users.

Since the hardening landed after 1.6.0 (1.6.1rc0 is the first tag containing it), 1.6.0 looks like the correct value: it is already released and strictly below every current build, so the warning fires on 1.6.1rc0, 1.6.1 and later. I confirmed version_leq(v, "1.6.0") is False for the current dev version.

Suggested change:

_HASH_TYPE_DEFAULT_CHANGE_VERSION = "1.6.0"

[garciadias]

In deprecated_arg_default, "replaced" means the version at which the default was or will be replaced, not the version at which the warning itself is retired. With replaced set to 1.8, the message a user sees on a 1.6.x or 1.7.x build is:

Current default value of argument hash_type="md5" has been deprecated since version 1.6.1. It will be changed to hash_type="sha256" in version 1.8.

But the default already is sha256 today, on line 183 of this same file. Someone who has just hit a HashCheckError reads "it will be changed in version 1.8" and reasonably concludes their breakage must be something else — the opposite of the guidance this PR is trying to give.

Setting replaced to the version where the change actually happened produces the correct wording, which reads "was changed in version from hash_type="md5" to hash_type="sha256"".

If we also want to communicate a horizon for removing the warning itself, that belongs in msg_suffix rather than in replaced. The constant name encodes the same misreading and is worth renaming alongside it.

Suggested change: pass the version in which the default actually changed as replaced=, and move any "warning removed in 1.8" note into the message suffix.

[garciadias]

old_default and new_default are being passed as strings that contain literal quote characters. That matters for more than cosmetics: deprecated_arg_default has a guard around line 310 of monai/utils/deprecate_utils.py that raises ValueError when the declared new_default already equals the function's actual default but "replaced" says the change is still in the future. Because the string '"sha256"' never equals "sha256", that guard is silently bypassed.

I checked both forms against this exact signature:

without quotes -> ValueError: Argument hash_type was replaced to the new default value sha256 before the specified version 1.8.
with quotes    -> no ValueError, guard bypassed

That ValueError is the utility correctly diagnosing the inconsistency I raised in the comment about replaced=1.8 above. Once replaced is corrected to the version where the default really changed, the unquoted values pass the guard legitimately and we keep the validation for the future.

The same applies to the three other decorator blocks in this file, at lines 223-224, 347-348 and 437-438.

Suggested change:

old_default="md5",
new_default="sha256",

[garciadias]

deprecated_arg_default warns purely on argument absence, before the function body runs. But hash_type only has any effect when a hash value is actually supplied: download_url consults it at lines 273 and 303, and extractall at line 387 behind an "if hash_val and ..." guard. So a call with hash_val=None warns about a hash algorithm that is never used. Confirmed on a build where the decorator is active:

download_url(hash_val=None)  ->  1 FutureWarning

That matters because MONAI itself calls these functions without hash_type and usually without a hash value at all. I found 17 such call sites: monai/bundle/scripts.py lines 199, 200, 206, 207, 216, 217, 263, 568 and 569; monai/networks/nets/hovernet.py 635 and 670; monai/networks/nets/senet.py 304; monai/networks/nets/swin_unetr.py 1319; monai/apps/auto3dseg/bundle_gen.py 431; monai/apps/tcia/utils.py 103; monai/transforms/utils_create_transform_ims.py 215. Once the version gating is fixed, every bundle download and every pretrained-weights fetch will print an MD5-to-SHA-256 migration notice that has nothing to do with what the user asked for. Warning noise at that scale tends to train people to ignore MONAI warnings, which undercuts the goal of this PR.

Two ways out. Either keep the decorator only on check_hash and let the others inherit it through the call chain — worth noting the chain does not currently double-warn, since download_and_extract forwards hash_type explicitly, and I measured exactly one warning for a fully defaulted download_and_extract call — or keep all four decorators and first make the internal call sites pass hash_type explicitly, so MONAI never warns about itself. The second is the smaller behavioural change and documents intent at each call site.

[garciadias]

This test was introduced by the hardening PR specifically to prove that the default hash type is sha256. Passing hash_type="sha256" explicitly makes the assertion tautological — it would still pass if someone reverted the default to "md5", which is exactly the regression the test exists to catch. Silencing the new warning by removing the coverage it was meant to protect is a net loss.

Better to keep the call implicit and handle the warning around it, for example:

with warnings.catch_warnings():
    warnings.simplefilter("ignore", FutureWarning)
    self.assertTrue(check_hash(filename, sha256))

or by asserting both the returned value and the warning in the same test. The same consideration applies to test_warns_when_val_is_none at line 87.

[garciadias]

This assertion is only true when the running MONAI version is past the "since" value, so its outcome depends on whether the checkout has git tags. On a tagged clone of this branch it fails with 0 != 1; on the same tree without .git it passes. Even after the version gating is corrected, it would be worth making that expectation explicit rather than incidental — deprecated_arg_default accepts a version_val parameter precisely so tests can pin it.

Patching the private helper _download_with_progress at lines 119, 125, 151, 165, 190 and 199 also couples these tests to an internal that could move.

Suggested change: construct the decorator under test with an explicit version_val, or add a visible version guard so the assumption is stated rather than inherited from the checkout.

[garciadias]

Thanks for picking this up @ericspod — using deprecated_arg_default rather than a hand-rolled warning is the right approach and matches what CONTRIBUTING.md asks for. My concerns are all about the version configuration rather than the design, and I have left them inline.

The blocking one: with since set to 1.6.1, deprecated_arg_default installs an identity decorator on every current build, because dev versions as 1.6.1rc0+N.g which orders strictly below 1.6.1. On a tagged checkout of this branch, check_hash with hash_type omitted emits zero warnings, and the new test fails with 0 != 1. CI is green only because the unit-test jobs check out shallow without tags, so the version resolves to 0+untagged and the utility substitutes sys.maxsize. In other words the passing checks here do not show that the warning reaches users. since=1.6.0 fixes it.

Two related points: replaced=1.8 makes the emitted message say the change is still upcoming when the default already is sha256, and quoting the default values as '"md5"' and '"sha256"' bypasses the guard in deprecate_utils that would have raised a ValueError naming that exact inconsistency. There is also a noise question — the warning fires on argument absence even when no hash value is passed, and 17 MONAI-internal call sites would then warn about MONAI itself.

One small thing on the description: the repository template opens with a "Fixes # ." line, which is missing here. The hardening PR is referenced in prose but there is no linked issue, so this compatibility follow-up is not discoverable from the tracker or from #9088. Worth adding a "Follow-up to #9088" reference in that slot.

Requesting changes on the version gating specifically — once the warning actually fires on the builds that carry the change, the rest is straightforward.

[ericspod]

Suggested change

	_HASH_TYPE_DEFAULT_CHANGE_VERSION = "1.6.1"
	_HASH_TYPE_DEFAULT_CHANGE_VERSION = "1.6.0"

[ericspod]

Closing this for #9130.