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

introduce sumfinite, enhancement to *finite, rename minfinite/maxfinite #17

Merged
merged 6 commits into from
Oct 11, 2021

Conversation

johnnychen94
Copy link
Member

@johnnychen94 johnnychen94 commented Sep 25, 2021

Features:

  • (new function) add sumfinite([f=identity], A)
  • add meanfinite([f=identity], A)
  • add minimum_finite([f=identity], A)
  • add maximum_finite([f=identity], a)

Bug fixes:

  • "fixes" varfinite for RGB array inputs using dot product. This requires ColorVectorSpace v0.9.7
  • support minimum_finite(A; dims) and maximum_finite(A; dims)

Deprecations:

  • rename: minfinite -> minimum_finite
  • rename: maxfinite -> maximum_finite
  • deprecate maxabsfinite(A) in favor of maximum_finite(abs, A)

There are some type infer issues, which I leave as future work.

Closes #14

Functionaility changes:

* support `meanfinite([f=identity], A)`
* add `sumfinite([f=identity], A)`
* "fixes" `varfinite` for `RGB` array inputs using dot product

Also add more comprehensive tests
* rename: `minfinite` -> `minimum_finite`
* rename: `maxfinite` -> `maximum_finite`
* add function argument to `minimum_finite` and `maximum_finite`
* deprecate `maxabsfinite(A)` in favor of `maximum_finite(abs, A)`
* support `minimum_finite(A; dims)` and `maximum_finite(A; dims)
* more comprehensive tests
@codecov
Copy link

codecov bot commented Sep 25, 2021

Codecov Report

Merging #17 (a4111bf) into master (78fb34c) will increase coverage by 0.45%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #17      +/-   ##
==========================================
+ Coverage   89.94%   90.40%   +0.45%     
==========================================
  Files           6        6              
  Lines         189      198       +9     
==========================================
+ Hits          170      179       +9     
  Misses         19       19              
Impacted Files Coverage Δ
src/statistics.jl 82.14% <87.50%> (+5.95%) ⬆️
src/utils.jl 100.00% <100.00%> (ø)

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 78fb34c...a4111bf. Read the comment docs.

Copy link
Member

@timholy timholy left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for doing it!

Project.toml Outdated Show resolved Hide resolved
@@ -61,14 +83,14 @@ if Base.VERSION >= v"1.1"
function varfinite(A; kwargs...)
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this varmult_finite instead?

Copy link
Member Author

@johnnychen94 johnnychen94 Oct 11, 2021

Choose a reason for hiding this comment

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

A good suggestion, let me open an issue for it first and then fix it in a separate PR; this PR is already quite big actually.

Edit: issue opened #18

@johnnychen94 johnnychen94 merged commit 12a7795 into master Oct 11, 2021
@johnnychen94 johnnychen94 deleted the jc/xfinite branch October 11, 2021 12:44
johnnychen94 added a commit to JuliaImages/Images.jl that referenced this pull request Oct 31, 2021
This commit moves some of the basic stats-related functions to ImageBase so
that other packages don't need to depend on the large Images dependency.

A few deprecations are involved to support the more generic version. See also:
JuliaImages/ImageBase.jl#17

Co-authored-by: JohnnyChen <[email protected]>
johnnychen94 added a commit to johnnychen94/Images.jl that referenced this pull request May 21, 2022
This commit moves some of the basic stats-related functions to ImageBase so
that other packages don't need to depend on the large Images dependency.

A few deprecations are involved to support the more generic version. See also:
JuliaImages/ImageBase.jl#17

Co-authored-by: JohnnyChen <[email protected]>
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.

maxabsfinite is breaking
2 participants