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

Avoid redundant full-cube-passes when region submasking #801

Merged
merged 5 commits into from
May 17, 2022

Conversation

keflavich
Copy link
Contributor

Region sub-masking has been using minimal_subcube, which shouldn't be necessary b/c we already downselect to the minimal bounding box.

Also, it was creating 3D masks when only 2D is needed - the broadcasting might therefore not have been done lazily.

@codecov-commenter
Copy link

codecov-commenter commented Mar 19, 2022

Codecov Report

Merging #801 (122397f) into master (f92fe7d) will decrease coverage by 0.03%.
The diff coverage is 16.66%.

❗ Current head 122397f differs from pull request most recent head 8664b89. Consider uploading reports for the commit 8664b89 to get more accurate results

@@            Coverage Diff             @@
##           master     #801      +/-   ##
==========================================
- Coverage   77.94%   77.90%   -0.04%     
==========================================
  Files          24       24              
  Lines        5853     5856       +3     
==========================================
  Hits         4562     4562              
- Misses       1291     1294       +3     
Impacted Files Coverage Δ
spectral_cube/analysis_utilities.py 83.83% <ø> (ø)
spectral_cube/spectral_cube.py 76.59% <16.66%> (-0.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f92fe7d...8664b89. Read the comment docs.

@keflavich keflavich force-pushed the faster_region_masking branch from a3fd542 to b62dda8 Compare May 16, 2022 12:46
@keflavich
Copy link
Contributor Author

This produces WCS mismatches only on windows. That seems unlikely; either this is a problem with the test grid or a genuine problem with how WCS behaves on windows machines?

@keflavich keflavich requested a review from e-koch May 17, 2022 13:34
@keflavich
Copy link
Contributor Author

Ready for final review

Copy link
Contributor

@e-koch e-koch left a comment

Choose a reason for hiding this comment

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

Small docs change. Otherwise looks good!

spectral_cube/spectral_cube.py Outdated Show resolved Hide resolved
@e-koch e-koch merged commit 39addbe into radio-astro-tools:master May 17, 2022
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.

3 participants