search

nignatiadis / SmoothingSplines.jl

Cubic smoothing splines in Julia.
Other
34 stars 14 forks source link

Updated one signature to accept ranges as well. #3

Closed mauro3 closed 7 years ago

mauro3 commented 7 years ago

Relaxed one type signature. Note that many more should probably be relaxed to at least AbstractArray.

mauro3 commented 7 years ago

Bump. Any change to get this merged?

nignatiadis commented 7 years ago

Sorry for replying so late; end of quarter period was super busy and then I forgot about this.

Regarding the PR: Some of the matrix algorithms use LAPACK (and right now only work with LAPACKFloat), so I would like to keep it in the type signature. Would something like AbstractArray{LAPACKFloat} also work for you? Or do you think allowing xs arbitrary would not possibly cause problems (since you know a lot more about Julia than I do)?

mauro3 commented 7 years ago

No worries. Yes, AbstractVector should be fine too. I think in general it's better to have a less strict signature with some danger of causing a runtime error, than too strict and thus not letting a user do something which ought to be possible.

Should I also change Vector -> AbstractVector elsewhere to be consistent?

nignatiadis commented 7 years ago

Ah makes sense! If you want to/think it would be useful, then sure.

mauro3 commented 7 years ago

I changed signatures to use AbstractArray elsewhere too.

nignatiadis commented 7 years ago

Great, thank you a lot! Do you want this to be tagged in metadata?

mauro3 commented 7 years ago

That would be great, thanks! (BTW you will probably prompted to put minimal version in your REQUIRE file. So, better do that before submission.)

nignatiadis commented 7 years ago

Hi again. Sorry I am being so slow again, but deadline after deadline... I have added you as a collaborator if you want to be able to make straightforward changes to the code or push yourself to metadata. Else I will do it sometime soon.