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

fix typing of UnivariateZeroState init #208

Merged
merged 1 commit into from
Jan 15, 2021
Merged

Conversation

Gyoshi
Copy link
Contributor

@Gyoshi Gyoshi commented Jan 15, 2021

eltype(x1)[] is incorrectly used here instead of typeof(x1)[]. Does not create a problem for most cases since eltype(x) == typeof(x) when x is a scalar.

@codecov
Copy link

codecov bot commented Jan 15, 2021

Codecov Report

Merging #208 (a8fe4b0) into master (fb83ad1) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #208   +/-   ##
=======================================
  Coverage   88.98%   88.98%           
=======================================
  Files           9        9           
  Lines        1461     1461           
=======================================
  Hits         1300     1300           
  Misses        161      161           
Impacted Files Coverage Δ
src/derivative_free.jl 89.64% <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 fb83ad1...a8fe4b0. Read the comment docs.

@jverzani
Copy link
Member

Many thanks! While I have your attention, any thought on how to widen the type signature beyond a union of Tuple and Vector to include any iterable of length 2?

@jverzani jverzani merged commit a312ef8 into JuliaMath:master Jan 15, 2021
@Gyoshi
Copy link
Contributor Author

Gyoshi commented Jan 16, 2021

Ok, I am hardly an authority on the matter, but how about not explicitly requiring an iterator, and just using it as if it is one--letting the code throw a MethodError if getindex or iterate is not defined?
JuliaLang/julia#23429 (comment)
If you want to be strict in terms of length, I would just @assert length(arr) == 2, but of course you lose a bit of generality since you are requiring that length has a method defined for that iterator.

If you need to specify that the eltype needs be the same in the type signature (for an arbitrary type of the iterator), you could look into WhereTraits.jl, but I'm not convinced that is a good idea.

@Gyoshi Gyoshi deleted the gyoshi branch January 16, 2021 01:23
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