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

Make minimum/maximum generic for Frequencies #54

Merged
merged 2 commits into from
Mar 1, 2021

Conversation

platawiec
Copy link
Contributor

Before:

julia> import Unitful: mm
julia> using FFTW
julia> fftfreq(10, 1.0mm);
julia> minimum(f)
ERROR: DimensionError: 0.0 and 0.1 mm are not dimensionally compatible.

After:

julia> import Unitful: mm
julia> using FFTW
julia> fftfreq(10, 1.0mm);
julia> minimum(f)
-0.5mm

I didn't include a test, but if you are ok w/ including Unitful in test dependencies I can put that in.

@platawiec platawiec changed the title Make generic comparison Make minimum/maximum generic for Frequencies Feb 26, 2021
@codecov
Copy link

codecov bot commented Feb 26, 2021

Codecov Report

Merging #54 (7794eaa) into master (193d035) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #54   +/-   ##
=======================================
  Coverage   52.63%   52.63%           
=======================================
  Files           2        2           
  Lines          95       95           
=======================================
  Hits           50       50           
  Misses         45       45           
Impacted Files Coverage Δ
src/definitions.jl 52.12% <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 193d035...7794eaa. Read the comment docs.

@timholy
Copy link
Member

timholy commented Feb 27, 2021

This is great. Just needs a test, and it would be fine to add a test dependency.

@stevengj
Copy link
Member

@timholy
Copy link
Member

timholy commented Feb 27, 2021

Presumably one can manually load testhelpers.jl via Sys.BINDIR, but it's a little ugly.

@stevengj
Copy link
Member

stevengj commented Feb 28, 2021

We can use Unitful.jl as a test dependency for now. If JuliaLang/julia#39852 is merged we can switch to that in a future release.

@timholy timholy merged commit 5fb4b43 into JuliaMath:master Mar 1, 2021
@timholy
Copy link
Member

timholy commented Mar 1, 2021

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

Successfully merging this pull request may close these issues.

3 participants