Adding all basics for spectroscopy

[briederer]

I've added a struct for collecting Quantities with errors (statistic and systematic). The Constructors allow to create the structs in a rather intuitive way using either several versions of the Quantity()function or the ±-Operator.

Please @fzierler have a look at possible optimisations here before I go on to use these structs and writing statistics functions which return those.

[github-actions[bot]]

The Docs for this PR are deployed! 🥳 You can find them here.

Please make sure that you have documented your new feature properly:

• [ ] Added documentation (if needed).

[briederer]

Also why is codecov failing?

[fzierler]

It's not failing - you still can read the correct report. I think it only shows that this PR reduces coverage since it introduces code that is not being used in tests.

[briederer]

Ah so codecov says it's failing when coverage decreases. Thanks for clarification. I planned to add the tests and docs in the end before merging.

[briederer]

The only thing we still need to think of is what to do when someone calls the constructor with DataPoint(::Integer, ::Float) or other mixed type variations. In terms of errors the right thing to do would be to ceil() the errors to the expected datatype. Any opinions on that?

[fzierler]

I would just add another layer where we promote to the correct type. Something like

function DataPoint(x::Number,sys::Tuple,stat::Tuple)
    T = promote_type(typeof(x),eltype(sys),eltype(stat))
    DataPoint(T(x),T.(sys),T.(stat))
end

[briederer]

Very elegant solution. And I guess we leave it up to the user to check for correctness without using any ceil() if x is an Integer. I'll add

convert(::Type{DataPoint{T}}, D::DataPoint) where {T <: Integer}

which handles this conversion explicitly.

[fzierler]

Very elegant solution. And I guess we leave it up to the user to check for correctness without using any ceil() if x is an Integer. I'll add

convert(::Type{DataPoint{T}}, D::DataPoint) where {T <: Integer}

which handles this conversion explicitly.

I am unsure if I understand the problem that is addressed by this explicit conversion. Could you please elaborate?

[briederer]

I am unsure if I understand the problem that is addressed by this explicit conversion. Could you please elaborate?

Sure. If you have for instance a measured value 1 and corresponding the statistical error 0.1 then the resulting DataPoint would be 1.0 ± 0.1. This is however nonsensical since you can't have a Float-Error for an Integer-Datapoint and right way to deal with this is to return 1 ± 1 i.e. increasing the error to the next higher integer value. And with the convert function this should be done in my opinion.

[fzierler]

I am unsure if I understand the problem that is addressed by this explicit conversion. Could you please elaborate?

Sure. If you have for instance a measured value 1 and corresponding the statistical error 0.1 then the resulting DataPoint would be 1.0 ± 0.1. This is however nonsensical since you can't have a Float-Error for an Integer-Datapoint and right way to deal with this is to return 1 ± 1 i.e. increasing the error to the next higher integer value. And with the convert function this should be done in my opinion.

I see, thank you. Maybe in that case

function DataPoint(x::T,sys,stat) where T<:Integer
    DataPoint(x,ceil.(T,sys),ceil.(T,stat))
end

would work for arbitrary Integer-Types. Let's hope this does not introduce any ambiguity errors - at least we should write tests for all sorts of datatype combinations. I will look into this later today.

[briederer]

You are right. Probably it's better to have a constructor in that case that already does that, than a convert function.

[fzierler]

Still needs docs. If you want I can write-up the behaviour of the datatype and its constructors. I don't think that we will need docs for anything else, since we probably won't export it.

Edit. I think we should also explicitly import Base.show()since we ware extending it with our DataPoint-specific methods.

[briederer]

I am quite satisfied with the current stage of the struct in general.

If you agree also I would write further tests and the docs next week, as well as adding statistics and simple error-propagation functions.

Just wanted to ask if you know what the proper way is for overloading base functions. Just import them in LatSpec.jl and export Base again or not?

[fzierler]

I am quite satisfied with the current stage of the struct in general.

If you agree also I would write further tests and the docs next week, as well as adding statistics and simple error-propagation functions.

So am I. I think this PR needs only docs and a few tests (there seems to be still some method that is not yet tested according to codecov) and docs and is ready to merge.

Just wanted to ask if you know what the proper way is for overloading base functions. Just import them in LatSpec.jl and export Base again or not?

I think this is the proper way, but you don't have to export them.

[briederer]

So am I. I think this PR needs only docs and a few tests (there seems to be still some method that is not yet tested according to codecov) and docs and is ready to merge.

Yes, still haven't written the tests for the DataPoint-Constructors. And docs will be done soon (; I also want to add the basic arithmetic operators for this type before merging.

I think this is the proper way, but you don't have to export them.

Imported Base.show and Base.convert now.

[briederer]

And also I guess datapoint.jl is a way more instructive name than uncertainties.jl, so I will change it.

[briederer]

Did you mean to commit this to this PR? It seems better to have a separate PR for it since it is not directly related to spectroscopy.

Nope commit 86b4a0a slipped in here. I'll resolve this.

[fzierler]

Is this good to go?

[briederer]

Apart from missing docs and maybe testcases I have missed (which does not seem to be the case according to codecov), yes. But honestly I don't have time right now to write the docs. So we could either merge it without docs for now or if you have time you could add them.

[fzierler]

I have added some docs for DataPoint and included some exported helper functions for obtaining the fields of the type, e.g. value(x). This is cleaner for broadcasting compared to accessing the field using x.val or getfield(x,:val).

[briederer]

Apart from the one Doc-Issue feel free to merge then (:

[fzierler]

Apart from the one Doc-Issue feel free to merge then (:

OK, once CI passes I will merge. :)