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

for loop for azimuthal averaging in isotropic functions #70

Merged
merged 4 commits into from
Apr 10, 2019
Merged

Conversation

roxyboy
Copy link
Member

@roxyboy roxyboy commented Apr 10, 2019

I inserted a for loop for the isotropic functions so that it can take arrays up to 4 dimensions.

@rabernat
Copy link
Collaborator

I think we may be able to improve this in the future using my new xhistogram package.

For now it's fine as is.

@codecov-io
Copy link

codecov-io commented Apr 10, 2019

Codecov Report

Merging #70 into master will increase coverage by 0.82%.
The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
+ Coverage   95.66%   96.48%   +0.82%     
==========================================
  Files           2        2              
  Lines         692      712      +20     
  Branches      119      123       +4     
==========================================
+ Hits          662      687      +25     
+ Misses         16       14       -2     
+ Partials       14       11       -3
Impacted Files Coverage Δ
xrft/tests/test_xrft.py 98.67% <100%> (-0.02%) ⬇️
xrft/xrft.py 94.04% <90.9%> (+2.06%) ⬆️

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 8af5fc4...0a5c152. Read the comment docs.

Copy link
Collaborator

@rabernat rabernat left a comment

Choose a reason for hiding this comment

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

Just a few small comments.

xrft/xrft.py Outdated
for i in range(M[1]):
iso_ps[j,i] = _azimuthal_avg(kidx, f[j,i], area, kr)
else:
raise ValueError("Arrays with more than 4 dimensions is not supported.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

are not supported

xrft/xrft.py Outdated
for i in range(M[1]):
iso_cs[j,i] = _azimuthal_avg(kidx, f[j,i], area, kr)
else:
raise ValueError("Arrays with more than 4 dimensions is not supported.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you have this code repeated in two functions, you should probably break it into a private standalone function.

@roxyboy roxyboy merged commit afdf053 into master Apr 10, 2019
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