Experiment followup dlv #917
Conversation
Test Results 6 files 6 suites 7m 25s ⏱️ Results for commit 3229266. ♻️ This comment has been updated with latest results. |
|
What is the purpose of the ordering (i.e. not |
|
Without remembering details I would expect some error occuring when (ab)using this class in the modified linspacebuilder to be the reason - perhaps something like an is_close-check (at least that's where abs() was e.g. called) where this is implicitly called and the result does not relly matter and one can just return False always. Or just leave it out and see what happens, as the definition is not very logical indeed. |
|
I would leave it out and move the hack to the location where a potential error occurs. Otherwise, this can lead to very weird and hard to debug bugs in the future. |
|
agreed, then omit that for now |
qupulse/program/values.py
Outdated
| @@ -86,7 +104,10 @@ def __rmul__(self, other): | |||
| def __truediv__(self, other): | |||
| inv = 1 / other | |||
| return self.__mul__(inv) | |||
|
|
|||
|
|
|||
| def __hash__(self): | |||
There was a problem hiding this comment.
DynamicLinearValue is not immutable. Either we make it immutable or we remove hash
|
sympy doesn't seem to like the changes |
…obably also better performance.
qupulse/program/values.py
Outdated
|
|
||
| #this is not to circumvent float errors in python, but rounding errors from awg-increment commands. | ||
| #python float are thereby accurate enough | ||
| def __call__(self, resolution: Optional[float]) -> Union[NumVal,TimeType]: |
There was a problem hiding this comment.
Can me make this a method with a proper name?
There was a problem hiding this comment.
one could of course
But since this functionality was the main purpose of the class, I thought it would be most intuitive to mimic a function called with the given resolution instead of having a name overhead by making it a named method
There was a problem hiding this comment.
A reader who is not aware of this class will be very puzzled. In my (non-humble) opinion making a callable class should be reserverd for cases where you need a callable anyways but need class semantics on top. A variable of such a class should then be named like a function. In this case the variable will (likely?) be named according to the context and function that the value has. The effect of calling the value will be opaque to the reader unless he looks up the class.
|
The tests fail due to coveralls unreliability. I will create a seperate PR to fix that. |
|
@Nomos11 merge if you agree with my changes |
No description provided.