-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix network_type docstring formatting in PerceptualLoss #8670
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
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThe enum class in monai/losses/perceptual.py was renamed from Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
monai/losses/perceptual.py (1)
75-75: Update type annotation to match accepted types.Type annotation specifies
strbut should bestr | PercetualNetworkTypeto match the docstring and actual usage.🔎 Proposed fix
- network_type: str = PercetualNetworkType.alex, + network_type: str | PercetualNetworkType = PercetualNetworkType.alex,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
monai/losses/perceptual.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/losses/perceptual.py
🔇 Additional comments (2)
monai/losses/perceptual.py (2)
52-56: Docstring update is accurate but type annotation doesn't match.The docstring correctly documents that
network_typeaccepts bothstrandPercetualNetworkType, but the type annotation on line 75 only specifiesstr.
98-102: No issues found.StrEnummembers inherit fromstr, so.lower()works on enum inputs and comparison againstlist(PercetualNetworkType)succeeds for both string and enum inputs. Logic is correct as written.
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit. Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit. Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
monai/losses/perceptual.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/losses/perceptual.py
🧬 Code graph analysis (1)
monai/losses/perceptual.py (1)
monai/utils/enums.py (1)
StrEnum(68-90)
🔇 Additional comments (1)
monai/losses/perceptual.py (1)
52-60: LGTM: Improved docstring clarity.The reformatted list of valid
network_typevalues is clearer than the previous inline notation.
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: 7ce29a1 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: cc0c951 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: 7ce29a1 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: cc0c951 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: 3e92d74 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
|
Hi @virginiafdez, The spelling/enum issue in PerceptualLoss has been fixed (PercetualNetworkType → PerceptualNetworkType), and all references/docstrings have been updated accordingly. Additionally, all missing DCO sign-offs have been added via remediation commits. Everything should now be in order for review. |
Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
e --abort
e --abort
Outdated
| [33mbc38c962[m[33m ([m[1;36mHEAD[m[33m -> [m[1;32mdev[m[33m)[m Fix PerceptualLoss enum references and docstrings | ||
| [33mbe2869df[m Fix network_type docstring formatting in PerceptualLoss | ||
| [33mccea8930[m[33m ([m[1;31morigin/dev[m[33m, [m[1;31morigin/HEAD[m[33m)[m DCO Remediation Commit for Mohammad Amanour Rahman <amanourrahman@gmail.com> | ||
| [33m0c667a99[m DCO Remediation Commit for Mohammad Amanour Rahman <amanourrahman@gmail.com> | ||
| [33mea64afaf[m DCO Remediation Commit for Mohammad Amanour Rahman <amanourrahman@gmail.com> | ||
| [33m3e92d74e[m Fix PerceptualLoss enum references and docstrings | ||
| [33m84e7d704[m DCO Remediation Commit for Mohammad Amanour Rahman <amanourrahman@gmail.com> | ||
| [33meeef1729[m DCO Remediation Commit for Mohammad Amanour Rahman <amanourrahman@gmail.com> | ||
| [33mcc0c951f[m Fix PerceptualLoss enum references and docstrings | ||
| [33m96ea131f[m DCO Remediation Commit for Mohammad Amanour Rahman <amanourrahman@gmail.com> | ||
| [33me2d0a027[m DCO Remediation Commit for Mohammad Amanour Rahman <amanourrahman@gmail.com> | ||
| [33m7ce29a1e[m Fix network_type docstring formatting in PerceptualLoss | ||
| [33m15fd428c[m Bump actions/checkout from 4 to 6 (#8649) | ||
| [33me5417c86[m Bump al-cheb/configure-pagefile-action from 1.4 to 1.5 (#8648) | ||
| [33mc1d02cf1[m Bump peter-evans/slash-command-dispatch from 4.0.0 to 5.0.0 (#8650) | ||
| [33me2677053[m Added an optional_import check for onnxruntime and applied the @unitt… (#8641) | ||
| [33m04a89ee9[m Generate heatmap transforms (#8579) | ||
| [33mf493ecdd[m 8620 modulenotfounderror no module named onnxscript in test py3x 311 pipeline (#8638) | ||
| [33m9a456274[m Fix #8350: Clarify LocalNormalizedCrossCorrelationLoss docstring (#8639) | ||
| [33m865b0e70[m Update documentation links (#8637) | ||
| [33mc968907c[m Fix #8599: Add track_meta and weights_only arguments to PersistentDataset for MetaTensor support. (#8628) | ||
| [33m806c0e84[m Fix index using tuple for image cropping operation (#8633) | ||
| [33m23c271ea[m feat: add activation checkpointing to unet (#8554) | ||
| [33m16e469c8[m Correct H&E stain ordering heuristic in ExtractHEStains (#8551) | ||
| [33ma8a7e575[m timestep scheduling with np.linspace (#8623) | ||
| [33mf315bcb0[m 8620 ModuleNotFoundError: No module named \'onnxscript\' in test-py3x (3.11) pipeline (#8621) | ||
| [33me72145ce[m Fix box_iou returning 0 for floating-point results less than 1. #8369 (#8553) | ||
| [33m579cec52[m added ReduceTrait and FlattenSequence (#8531) | ||
| [33md260b785[m Bump github/codeql-action from 3 to 4 (#8616) | ||
| [33mb10410ce[m Bump actions/upload-artifact from 4 to 5 (#8615) | ||
| [33m69444c13[m Bump actions/download-artifact from 5 to 6 (#8614) | ||
| [33md9cfa3e6[m Bump peter-evans/create-or-update-comment from 4 to 5 (#8612) | ||
| [33m09ace633[m Include more-itertools in build env (#8611) | ||
| [33m42a99477[m 8564 fourier positional encoding (#8570) | ||
| [33m69f3dd26[m Bump actions/setup-python from 5 to 6 (#8589) | ||
| [33mc4a1acc8[m Bump actions/checkout from 4 to 5 (#8588) | ||
| [33m8a2efcb0[m Bump actions/download-artifact from 4 to 5 (#8590) | ||
| [33m53382d83[m Update citation for 1.5.1 (#8582) | ||
| [33m9c6d819f[m Release 1.5.1 Updates (#8575) | ||
| [33m9122739e[m Removing instantiation in forward (#8580) | ||
| [33mcf5790db[m Fix gdown fails (#8576) | ||
| [33mcf5505a5[m Diffusion Model Encoder has an output layer set in forward method and this leads to problems (#8578) | ||
| [33m6327a861[m Transformers version (#8574) | ||
| [33m946cfdff[m Path traversal issue security fix (#8568) | ||
| [33m948fbb70[m Torch and Pickle Safe Load Fixes (#8566) | ||
| [33m401ea4a0[m Fix hardcoded input dim in DiffusionModelEncoder (#8514) | ||
| [33mfd13c1b5[m Create SECURITY.md (#8546) | ||
| [33m725c8dea[m Torchvision pretrain fix (#8563) | ||
| [33mb5bc69d0[m Classifier free guidance unconditioned value (#8562) | ||
| [33m0968da20[m Improve Orientation transform to use the "space" (LPS vs RAS) of a metatensor by default (#8473) | ||
| [33md4ba52e3[m Fix build failure by pinning pyamg to versions below 5.3.0 (#8548) | ||
| [33mcb200af0[m Tests Cleanup (#8535) | ||
| [33m1e6c6615[m 8525 improve documentation on the datalist format (#8539) | ||
| [33mcf869814[m fix: HistogramNormalized doc (#8543) | ||
| [33mcafc1feb[m Pytorch 2.8 Support (#8530) | ||
| [33mb92b2ce8[m Add support for optional conditioning in PatchInferer, SliceInferer, and SlidingWindowInferer (#8400) | ||
| [33m2a12c4b4[m Add input validation to ImageStats class (#8501) | ||
| [33mede6ace8[m Throw exception on invalid images in retinanet detector (#8515) | ||
| [33m1dc0e68c[m 8185 test refactor 2 (#8405) | ||
| [33m503b4ede[m fix bug in maisi vae (#8517) | ||
| [33m8ee3f89b[m Updates for Pytorch 2.7 (#8429) | ||
| [33mdef42221[m Adding .coderabbit.yaml File (#8513) | ||
| [33me0be5d27[m Update README badges to add research paper citations number (#8494) | ||
| [33me499362b[m add kwargs in array and functional file (#8508) | ||
| [33m4b69748a[m ci: add custom timeout to ci job in order to save resources (#8504) | ||
| [33md388d1c6[m Update citation (#8484) | ||
| [33m3d6021a6[m Fix: ZarrAvgMerger ValueError with zarr_format 3 (#8477) | ||
| [33md38c93f9[m Removed or Replaced Broken URLs (#8483) | ||
| [33me5c512ff[m Fix integration test in 24.10 (#8480) | ||
| [33m423ea748[m Fixing URLs in README.md (#8481) | ||
| [33mdfe6fc77[m Changelog For 1.5 (#8474) | ||
| [33mc3a317d2[m Adding CODEOWNERS (#8457) | ||
| [33mc3a3397d[m Remove unused test cases in bundle load (#8463) | ||
| [33m2d416557[m Classifier free guidance (#8460) | ||
| [33mf85135b0[m Remove deprecated `net_name` in test file (#8461) | ||
| [33md4b4bff9[m Adding Support Policy + Doc Updates (#8458) | ||
| [33m222d5094[m Migrate to modern Python Logger API (#8449) | ||
| [33mbf6bc1fd[m Enable code coverage comments on PRs in codecov configuration (#8402) | ||
| [33m707b2305[m Remove deprecated `return_state_dict ` in bundle `load` (#8454) | ||
| [33m03a0d851[m Fix AutoencoderKL docstrings. (#8445) | ||
| [33mdaf9f054[m Update default overlap value in occlusion_sensitivity to 0.6 (#8446) | ||
| [33mf97a0e9b[m Remove deprecated functionality for v1.5 (#8430) | ||
| [33m4305bb81[m :bug: fix cosine noise scheduler (#8427) | ||
| [33m0d19a72f[m Temporarily Restrict setuptools Version to 79.0.1 (#8441) | ||
| [33mb58e883c[m selfattention block: Remove the fc linear layer if it is not used (#8325) | ||
| [33mf27517b8[m Inverse Threading Fix (#8418) | ||
| [33m8f3d8e87[m Update get latest bundle version function (#8420) | ||
| [33meadffd24[m Fix OptionalImportError: required package `openslide` is not installed (#8419) | ||
| [33m9586071c[m Add Skip test in TestTranschex (#8416) | ||
| [33m6c8de354[m 8394 Update bundle download API (#8403) | ||
| [33m83dcd35c[m Fix: correctly apply fftshift to real-valued data inputs (#8407) | ||
| [33m90ead4bf[m 8328 nnunet bundle integration (#8329) | ||
| [33m4986d7ff[m Auto3DSeg algo_template hash update (#8406) | ||
| [33mbfcb318b[m Restormer Implementation (#8312) | ||
| [33m8aef9a98[m Update Dice Metric Docs (#8388) | ||
| [33m5c5ca232[m Auto3DSeg algo_template hash update (#8397) | ||
| [33m0f5da113[m Auto3DSeg algo_template hash update (#8393) | ||
| [33m41d6c9c2[m 8201 Fix DataFrame subsets indexing in CSVDataset() (#8351) | ||
| [33m34f37973[m add prediction type for rflow scheduler (#8386) | ||
| [33m90de55b1[m Add rectified flow noise scheduler for accelerated diffusion model (#8374) | ||
| [33m7c26e5af[m Enable Pytorch 2.6 (#8309) | ||
| [33m1983f277[m Auto3DSeg algo_template hash update (#8378) | ||
| [33m2e391c82[m Bump torch minimum to mitigate CVE-2024-31580 & CVE-2024-31583 and enable numpy 2 compatibility (#8368) | ||
| [33ma09c1f08[m Update monaihosting download method (#8364) | ||
| [33mab075234[m Modify ControlNet inferer so that it takes in context when the diffus… (#8360) | ||
| [33ma7905909[m 8354 fix path at test onnx trt export (#8361) | ||
| [33md98f3481[m Solves path problem in test_bundle_trt_export.py (#8357) | ||
| [33maf54a17b[m Add Average Precision to metrics (#8089) | ||
| [33mb0ed2534[m Fix CommonKeys docstring (#8342) | ||
| [33mfb8c5bf7[m Removed outdated `torch` version checks from transform functions (#8359) | ||
| [33m960c59b4[m Fix `packaging` imports in version comparison logic (#8347) | ||
| [33m0a85eed2[m Inferer modification - save_intermediates clashes with latent shape adjustment in latent diffusion inferers (#8343) | ||
| [33m44add8d1[m Bump min torch to 1.13.1 to mitigate CVE-2022-45907 unsafe usage of eval (#8296) | ||
| [33me538f7fd[m Recursive Item Mapping for Nested Lists in Compose (#8187) |
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.
Wrong file provided - cannot review git log output.
The file appears to be a git log dump rather than source code. The filename "e --abort" suggests a malformed command. The PR objectives indicate changes to monai/losses/perceptual.py (enum rename from PercetualNetworkType to PerceptualNetworkType), but that file was not provided for review.
Please provide the actual source file with code changes.
🤖 Prompt for AI Agents
In e --abort around lines 1-114 the uploaded file is a git log dump (wrong file)
instead of the intended source; replace this with the actual modified source
file monai/losses/perceptual.py containing the PR changes, rename the misspelled
enum PercetualNetworkType to PerceptualNetworkType (update the enum definition,
all references, imports, and type hints), fix related docstrings and any
occurrences of the old name, run unit tests and linters, and amend the commit/PR
to include the corrected file so reviewers can verify the change.
DCO Remediation Commit for Mohammad Amanour Rahman <amanourrahman@gmail.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to the previous commits.
Fixes # .
Description
This pull request fixes incorrect formatting and clarifies the documentation
for the
network_typeargument inPerceptualLoss.The docstring now correctly reflects the supported network options
and matches the actual implementation.
Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.No functional behavior is changed.