Skip to content

Conversation

@garrettwrong
Copy link
Collaborator

Closes #578

@codecov
Copy link

codecov bot commented Feb 18, 2022

Codecov Report

Merging #583 (0679e86) into develop (11c3dca) will increase coverage by 0.00%.
The diff coverage is 88.88%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #583   +/-   ##
========================================
  Coverage    86.47%   86.48%           
========================================
  Files          108      108           
  Lines         8283     8271   -12     
========================================
- Hits          7163     7153   -10     
+ Misses        1120     1118    -2     
Impacted Files Coverage Δ
src/aspire/nufft/cufinufft.py 4.68% <0.00%> (+0.07%) ⬆️
src/aspire/nufft/pynfft.py 0.00% <0.00%> (ø)
src/aspire/storage/micrograph.py 82.35% <0.00%> (-0.34%) ⬇️
src/aspire/utils/misc.py 89.47% <ø> (-0.53%) ⬇️
src/aspire/operators/filters.py 96.29% <66.66%> (-0.02%) ⬇️
src/aspire/reconstruction/kernel.py 78.57% <80.00%> (ø)
src/aspire/abinitio/commonline_sync.py 97.72% <100.00%> (ø)
src/aspire/basis/basis.py 87.83% <100.00%> (ø)
src/aspire/basis/basis_utils.py 98.17% <100.00%> (-0.02%) ⬇️
src/aspire/basis/fb_2d.py 98.97% <100.00%> (ø)
... and 13 more

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 11c3dca...0679e86. Read the comment docs.

Copy link
Collaborator

@chris-langfield chris-langfield left a comment

Choose a reason for hiding this comment

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

Looked through for any mishaps your script might have made but found none. Also did an extra git grep to make sure we got all of them. Thanks!

@garrettwrong garrettwrong marked this pull request as ready for review February 18, 2022 19:42
@garrettwrong garrettwrong requested a review from janden as a code owner February 18, 2022 19:42
Copy link
Collaborator

@janden janden left a comment

Choose a reason for hiding this comment

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

Thanks.

Just out of curiosity, I suppose this

    assert statements in Python are sometimes optimized away by the compiler, and are for internal testing purposes.
    For user-facing assertions, we use this simple wrapper to ensure conditions are met at relevant parts of the code.

is in fact not the case?

@garrettwrong
Copy link
Collaborator Author

Thanks.

Just out of curiosity, I suppose this

    assert statements in Python are sometimes optimized away by the compiler, and are for internal testing purposes.
    For user-facing assertions, we use this simple wrapper to ensure conditions are met at relevant parts of the code.

is in fact not the case?

Well it is half true, so that is a fair question. Not true for this project. Technically asserts can be optimized away, but only if we optimize or a user intentionally optimizes our code with special flags before attempting to use it. This isn't something we do. It is akin to disabling logging/warnings via command line flags.

Another, seperate discussion... If there is an assert that is not for sanity check purposes, testing, or debugging, it should be converted to raise an exception.

@garrettwrong garrettwrong merged commit 8ada1e9 into develop Feb 21, 2022
@garrettwrong garrettwrong deleted the deprecate_ensure branch February 21, 2022 15:20
@janden
Copy link
Collaborator

janden commented Feb 21, 2022

Ok thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants