use only() instead of first()

[st--]

…for 1-"vectors" that were for the benefit of Flux.

#397 (comment) says this only works for Julia 1.4+, but we already depend on the Compat package, which backports only.

This has the advantage that it a) makes clearer which parameters are actually scalar and b) until #397 is ready to be merged, adds additional error-checking.

[devmotion]

#397 would remove basically all these occurrences of first. I am bit worried that this PR introduces many merge conflicts.

[theogf]

I would also be in favor of moving #397 forward, this looks like a lot of unnecessary overlapping

[codecov]

Codecov Report

Merging #403 (27bffad) into master (2d17212) will increase coverage by 0.00%.
The diff coverage is 91.42%.

@@           Coverage Diff           @@
##           master     #403   +/-   ##
=======================================
  Coverage   92.73%   92.74%           
=======================================
  Files          52       52           
  Lines        1198     1199    +1     
=======================================
+ Hits         1111     1112    +1     
  Misses         87       87           

Impacted Files	Coverage Δ
src/kernels/scaledkernel.jl	88.23% <50.00%> (ø)
src/transform/periodic_transform.jl	50.00% <50.00%> (ø)
src/basekernels/constant.jl	100.00% <100.00%> (ø)
src/basekernels/exponential.jl	100.00% <100.00%> (ø)
src/basekernels/fbm.jl	100.00% <100.00%> (ø)
src/basekernels/matern.jl	100.00% <100.00%> (ø)
src/basekernels/polynomial.jl	100.00% <100.00%> (ø)
src/basekernels/rational.jl	100.00% <100.00%> (ø)
src/distances/sinus.jl	81.81% <100.00%> (+1.81%)	⬆️
src/kernels/transformedkernel.jl	72.72% <100.00%> (ø)
... and 2 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 2d17212...27bffad. Read the comment docs.

[st--]

I prepared this because it seemed like #397 might take rather longer to be ready for merge as it's still WIP, and this one could be merged right away (assuming I didn't miss anything and the tests pass).
I'm happy to fix all the merge conflicts for you (by making a subbranch off dw/scalar_parameterhandling and cleaning up the merge of either this branch or master in there), if you like?

[st--]

In fact I've already fixed your merge conflicts here: https://github.com/JuliaGaussianProcesses/KernelFunctions.jl/compare/st/scalar_parameterhandling_clean_merge

[st--]

Just to clarify, I'm still in favour of moving #397 forward. I just think this would be a neat improvement in the meantime, without impeding #397.

[devmotion]

Even if the PR takes some weeks until it is merged (hopefully it doesn't, there's not much to implement and fix, mainly some discussions about the design), I don't think it's useful to introduce these merge conflicts. This PR doesn't seem to fix any bugs or pressing issues, so I think it's not worth the additional time and effort that someone has to spend.

[devmotion]

In fact I've already fixed your merge conflicts here:

I appreciate this but I would like to keep the git history in the PR clean. I know you spent time on this but I really don't see the need for this PR given that the other one will be merged reasonably soon and it does not fix any issues (also most/all vectors are constructed internally).

[st--]

I don't think it's useful to introduce these merge conflicts. This PR doesn't seem to fix any bugs or pressing issues, so I think it's not worth the additional time and effort that someone has to spend.

Well, it's something that's been bugging me for a long time, and I would consider it a bug that the following runs, instead of erroring:

julia> k = with_lengthscale(SqExponentialKernel(), [2, 3])
Squared Exponential Kernel (metric = Distances.Euclidean(0.0))
	- ARD Transform (dims: 2)

julia> k(4., 2.)
0.6065306597126334

whereas it errors on this branch.

I agree it's not really worth spending lots of time arguing back and forth, but I don't think there's any time involved in fixing any merge conflicts. I've already done so in a branch, and I'm happy to update the branch to make sure master merges cleanly into your future branch without any effort on your side! You can just do git checkout --ours -- anyways.

I appreciate this but I would like to keep the git history in the PR clean.

Could you help me gain a better understanding of a) what would be "unclean" by merging master into your branch and b) what value you get out of avoiding that merge ?

[st--]

@devmotion @willtebbutt bump

[devmotion]

I still don't think it's necessary and that the disadvantages (such as merge conflicts - BTW not only in the PR discussed above but also eg in the show PR) outweigh potential improvements which won't be needed anyway in the not-to-distant future.

[st--]

Could you make "not-too-distant future" a bit more precise ? And I'd still like to understand where you see the disadvantages - re merge conflicts, as I said before, I don't think it's a big hassle and I already volunteered to sort them all out for you...