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

DM-41902: Fix real space convolutions for small images #5

Merged
merged 3 commits into from Nov 29, 2023

Conversation

fred3m
Copy link
Collaborator

@fred3m fred3m commented Nov 29, 2023

No description provided.

Copy link

codecov bot commented Nov 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (13f254e) 94.84% compared to head (f609548) 94.84%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main       #5   +/-   ##
=======================================
  Coverage   94.84%   94.84%           
=======================================
  Files          38       38           
  Lines        4730     4736    +6     
=======================================
+ Hits         4486     4492    +6     
  Misses        244      244           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@erykoff erykoff left a comment

Choose a reason for hiding this comment

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

The PR needs a title along with the ticket number, otherwise looks good.

@@ -326,7 +326,7 @@ def convolve(self, image: Image, mode: str | None = None, grad: bool = False) ->
newshape[2] += kernel.image.shape[2] - image.shape[2]
_image = _pad(_image, newshape)
result = convolve(_image, kernel.image, self.convolution_bounds)
result = centered(_image, image.data.shape)
result = centered(result, image.data.shape) # type: ignore
Copy link

Choose a reason for hiding this comment

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

What is the # type: ignore about?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's because convolve returns either an ndarray or Fourier class. I could cast it to an ndarray but I find typing.cast to be clunky and I find it more readable to just ignore those types of mypy errors when I know what the code is doing.

@fred3m fred3m changed the title DM-41902 DM-41902: Fix real space convolutions for small images Nov 29, 2023
@fred3m fred3m merged commit ee10cc3 into main Nov 29, 2023
9 checks passed
@fred3m fred3m deleted the tickets/DM-41902 branch November 29, 2023 22:44
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.

None yet

2 participants