lmfit
Should we encourage direct `UFloat` instantiation?
In uncertainties<4.0 the convention has been to instantiate "numbers with uncertainty" using the ufloat factory method. This is a bit strange with respect to typical python conventions where you would instantiate the class directly. I think it was maybe done this way because in the previous architecture there were two classes users would end up with during uncertainties calculations. Variable objects resulted from the ufloat call, but AffineScalarFunc objects arose as a result of any mathematical manipulation of Variable objects.
However, in #262, the ufloat function returns a UFloat object and mathematical manipulations also result in UFloat objects. I would say it would be more in line with python convention if we started encouraging direct instantiation via the UFloat constructor and discouraging the ufloat function. We will always have to keep the ufloat convention because it so deeply ingrained in the code for anyone using uncertainties that I wouldn't ever suggest deprecating it.
My vote is that we SHOULD start encouraging UFloat and discouraging ufloat. I recommend a prominent comment in the documentation to this effect, and then using UFloat throughout the documentation and never using ufloat. Eventually we convert all the tests over to UFloat (except for 1 or 2 to prove that ufloat continues to be a faithful wrapper). If we think it is important the prominent note can reassure users that there are no plans to deprecate ufloat ever.
I updated the new UFloat API to accept only std_dev and not uncertainty anymore to make the public API for UFloat match the API for ufloat. https://github.com/lmfit/uncertainties/pull/262/commits/5d3be9aad9f5f265ce4873be0d8e47499d34a3a5. When doing mathematical operations the UCombo object is directly manipulated and constructed. I had to add in a slightly hacked alternate UFloat constructor function in ops.py to allow uncertainties internals the control they need while allowing the clean user-facing API on `UFloat.
I'm asking about this now because the result of this choice will have an impact on the examples and sentences used in the updated documentation that I'm going through now.
@jagerber48 I don't have a strong opinion. I agree we need to keep ufloat. I guess the argument for UFloat() is "make an instance of a class".
OTOH, everything in Python is an object. That is, calling function ufloat() is like saying "construct and return UFloat object from these arguments". Many libraries (numpy, scipy) use functions/methods to construct and return complex objects.
That is all to say that a = ufloat(10, 0.3) doesn't seem that odd to me.
Perhaps we can live with both uses?
Thanks for the response Newville. You said you don't have a strong preference so I don't think I need to "convince" you of anything. I'll just take the chance to respond.
I guess I find it strange since it is a thin wrapper that actually does nothing except pass the arguments through to the class constructor. Again, in the old architecture it provided a layer of isolation between the user and the complexity involving the difference between Variable and AffineScalarFunc. That is, users just knew to use the ufloat function to create the special uncertainties objects that they are familiar with. But there would be confusion about what object the user even had, e.g.
# master branch
from uncertainties import ufloat
x = ufloat(1, 0.1)
type(x) == type(1 * x)
# False
So there was a desire to shield the users from the actual classes underlying this distinction.
But in the new framework we will have
type(x) == type(1 * x)
# True
so now the ufloat wrapper serves literally no purpose except backwards compatibility.
I'm curious what examples you're thinking of from numpy and scipy for thin constructor functions rather than using class constructor/initialization directly. np.ndarray is the obvious one, but other obvious don't come immediately to my mind.
@jagerber48 I would view numpy.array(), numpy.arange(), numpy.linspace() all as "ndarray constructors". I think numpy doesn't even export a CamelCaseClasses. Most of scipy is a collection of functions.
I would say that CamelCase for classes is just a convention in Python. Convention is a fine thing. And so is the fact that Python isn't so in-your-face with object orientation. Everything is an object, so an instance of a class. But there are plain functions that return (and maybe create) an object.
That's just: both are OK.... and it's great that ufloat()will be just a thin wrapper around UFloat!
... for "functions with lowercase names that return complex objects," see also: all of datetime and pytz.timezone().
Yeah those are good examples. Well, I think my plan is to quietly change the online documentation to using UFloat instead of ufloat everywhere. But I will have a section early on pointing out that users can continue to use the ufloat function to create UFloat objects as well, maybe with an assurance that there are no plans to ever remove ufloat. I also won't express any opinion about which options users should use (though I do personally have one :) ). I'll leave this issue open for you and other to weigh in more, but maybe the next steps is to see what the documentation looks like and discuss it then.
I'm OK with encouraging "UFloat" starting with v4 and favoring it heavily in the docs and examples.
Maybe deprecate ufloat for version 5 (i.e., "unseeable future")? I have no strong opinion on that.
I like switching to UFloat, meaning recommending using the class to construct the class. I think that better meets user expectations when there is not a reason to avoid (you can use isinstance for example).
Case-wise I think ufloat was meant to mirror float as many older Python classes like float predate the recommendation to use camel case. I don't have a strong feeling about that. We could even switch ufloat to being the class name and I would be okay with it.
The one thing the ufloat function provides is the warning about std_dev==0. That warning is nice because we split things so that ufloat is the user facing constructor and UFloat is the internal constructor currently. If we had only one constructor, I think I would prefer not to have the warning so that calculation does not generate it, though maybe it would not come up in practice.
I'm OK with encouraging "UFloat" starting with v4 and favoring it heavily in the docs and examples. Maybe deprecate
ufloatfor version 5 (i.e., "unseeable future")? I have no strong opinion on that.
I have no strong opinion about that either.
@wshanks thanks, those are all really good points.
Actually, in response to this issue I made some modifications to the UFloat.__init__() method that are relevant. Previously the signature was
def __init__(nominal_value: float, uncertainty: Union[float, UCombo])
where ufloat or users would use float input to the uncertainty but ops.py would pass UCombo input. However, I didn't want users to see UCombo anywhere. Basically I don't want that to be part of the public API. So I made a change where the signature is now
def __init__(nominal_value: float, std_dev: float)
so that users can call it directly and see a nice API. Then its ops.py I do what might be considered a trick or hack (we should hash this out in code review if we're ok with it) using object.__new__(UFloat) to bypass the __init__ method and inject the UCombo directly. Here's the code on the commit where the change was made https://github.com/lmfit/uncertainties/pull/262/commits/5d3be9aad9f5f265ce4873be0d8e47499d34a3a5#diff-9c181188b5fa4ab96ce2591b42539ab903f0c0b5fff1cfaf253a893b6b3aada6R346
The upshot of this is that we can put the warning into UFloat.__init__ and it will show up when users create UFloat but it won't show up when UFloat is created as a result of a mathematical operation.
Then finally what you say about the lower case vs camel case. With all of these changes we can actually just make ufloat an alias for UFloat. Then instead of having a maximally thin function wrapper we just have a legacy name alias which is more tolerable to me.