Change or remove @equivalence?

[sostock]

@equivalence does not do much more than @eqrelation, it just defines the struct before adding the relation. That doesn’t seem like it provides much value. I see two options:

1. Change @equivalence to only define the struct. Users would then write

   @equivalence MassEnergy
   @eqrelation MassEnergy Unitful.Energy/Unitful.Mass = Unitful.c0^2

   However, it doesn’t seem like this version of @equivalence provides much value either.
2. Just remove @equivalence, the user would then write

   struct MassEnergy <: Equivalence end
   @eqrelation MassEnergy Unitful.Energy/Unitful.Mass = Unitful.c0^2

I currently prefer option 2.

[rmsrosa]

I was in fact working on this yesterday, but i didn't quite got what i wanted.

I was trying to include @eqrelation in @equivalence, and have it figure out whether or not the equivalence exists or not. If it does, then proceeds as in current @eqrelation , otherwise, proceeds as in current @equivalence. But it was having problems with the docstring. i will try a little more, but if i do not succeed, i would say i prefer option 1.

[sostock]

I was trying to include @eqrelation in @equivalence, and have it figure out whether or not the equivalence exists or not.

Figuring out whether the type exists or not seems not easily possible. @isdefined only works if the type is defined/imported in the current module, it won’t work with a module qualifier (e.g., UnitfulEquivalences.MassEnergy). I am also not a big fan of doing different things depending on whether a variable is defined or not.

But it was having problems with the docstring.

Do you mean attaching the docstring to the struct in the macro? That’s what Base.@__doc__ is for.

i will try a little more, but if i do not succeed, i would say i prefer option 1.

Okay, then I will implement option 1.

[rmsrosa]

Well, i think i managed to do that with

macro equivalence(name, relation)
    quote
        try
            isa($name, DataType)
            $(_eqrelation(name, relation))
        catch
            Base.@__doc__ struct $(esc(name)) <: Equivalence end
            $(_eqrelation(name, relation))
        end
    end
end

and then we don't need to bother whether a relation has been defined or not and don't need to bother whether to use @equivalence or @eqrelation (with which i was quite confused at first). We can just write:

"""
    Spectral

Equivalence that relates the energy of a photon to its frequency and wavelength according to
the relation ``E = hf = hc/λ``, where
* ``E`` is the photon energy,
* ``f`` is the frequency,
* ``λ`` is the wavelength,
* ``h`` is the Planck constant and
* ``c`` is the speed of light in vacuum.

!!! Note
    The `Spectral` equivalence does not include the wavenumber. This is to avoid mistakes,
    since there are two competing definitions of wavenumber (``1/λ`` and ``2π/λ``).
"""
@equivalence Spectral Energy/Frequency  = h
@equivalence Spectral Energy*Length     = h*c0
@equivalence Spectral Length*Frequency  = c0

This worked well with the test.

Do you think this is "stable", i mean, do you think it will suffer from the same problem that isdefined has, that you mentioned?

I like this way, so the user just need to bother with one macro, but if you think option 1 is better (or safer), it is fine with me.

[sostock]

macro equivalence(name, relation)
    quote
        try
            isa($name, DataType)
            $(_eqrelation(name, relation))
        catch
            Base.@__doc__ struct $(esc(name)) <: Equivalence end
            $(_eqrelation(name, relation))
        end
    end
end

This version does work, but I would still prefer one of the two options above. Some comments on your solution:

• isa($name, DataType) is unnecessary, the subsequent method definition (in _eqrelation) will also throw an error if $name is undefined.
• The catch block is also executed if _eqrelation in the try block fails due to a wrong relation. In that case, the code will throw an error that the $name type is redefined instead of the error that relation is wrong, which is the actual problem. I think this is not optimal and I would generally not use try … catch for something like this.
• $name needs to be escaped in isa($name, DataType).

Since you are ok with option 1, I would merge #3.

[rmsrosa]

Ok, I understand the problems with my solution. Let's go with option 1, then!