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

Added min and median combine_functions #336

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

minzastro
Copy link

I add two new combine functions. One is simply 'min' - very similar to what exists already.
The other one is 'median', which required some per-block processing. This is probably slower - I made no tests for performance yet.

@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Merging #336 (05a9646) into main (185dae1) will increase coverage by 0.38%.
The diff coverage is 98.03%.

❗ Current head 05a9646 differs from pull request most recent head 298424c. Consider uploading reports for the commit 298424c to get more accurate results

@@            Coverage Diff             @@
##             main     #336      +/-   ##
==========================================
+ Coverage   94.27%   94.65%   +0.38%     
==========================================
  Files          24       24              
  Lines         803      842      +39     
==========================================
+ Hits          757      797      +40     
+ Misses         46       45       -1     
Impacted Files Coverage Δ
reproject/mosaicking/coadd.py 96.19% <98.03%> (+3.76%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@astrofrog
Copy link
Member

Thanks for the pull request! I'll try and take a look at this soon! In the meantime, I noticed that some of the code has been re-formatted - could you run tox -e codestyle to format it back and commit the changes?

@minzastro
Copy link
Author

Thanks for the pull request! I'll try and take a look at this soon! In the meantime, I noticed that some of the code has been re-formatted - could you run tox -e codestyle to format it back and commit the changes?

My IDE was complaining that formatting style is non-standard, that's why I changed it. Was this non-standard style intended? I can change it back of course

@astrofrog
Copy link
Member

@minzastro - sorry for not getting back to this. Now that we use dask internally in a few places in reproject I think we should also consider using dask to handle the median calculation to avoid having to do all the block handling ourselves. I'll open an alternative PR to try this out.

@astrofrog
Copy link
Member

I'm running into issues using dask, have posted a question here: https://dask.discourse.group/t/constructing-a-sparse-dask-array-from-numpy-arrays/2187 - once there is a reply, we can decide how to proceed.

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