DSL Bring Up 1 #141
DSL Bring Up 1 #141
Conversation
…t mismatch the unit's registry
…sing them on init
| @@ -957,7 +957,7 @@ def debugging() -> bool: | |||
| return debugpy.is_client_connected() | |||
|
|
|||
|
|
|||
| class FuncSet[T, H: Hashable](collections.abc.Set[T]): | |||
| class FuncSet[T, H: Hashable](collections.abc.MutableSet[T]): | |||
There was a problem hiding this comment.
What you are trying to implement is a custom eq function instead of a custom hasher that is removing collisions.
Just add an extra arg eq that checks if two elements (with the same hash) are equivalent and if so dont add it to the set.
Then no need for the discard stuff.
There was a problem hiding this comment.
I was using discard in the DSL front-end to keep track of the params that'd been referenced w/o declaration.
Maybe there's something else fucky with this implementation?
| @@ -14,6 +14,7 @@ | |||
|
|
|||
| UnitsContainer = _UnitsContainer | str | |||
| Quantity = P.Quantity | |||
| Unit = P.Unit | |||
There was a problem hiding this comment.
This is different than above. The types are not the same.
Got to be careful here, this might break stuff. Check where Unit is used atm.
There was a problem hiding this comment.
Yep, changed this because it was breaking stuff in the front-end
[pseudocode]
from ... import Unit, Quantity
def _get_unit(name: str) -> Unit:
return Unit(name)
assert isinstanc(10 * _get_unit("mV"), Quantity) # <-- fails^ This (distilled down) is what'd fail in the front-end because the Quantity and Unit were from different unit registrys
| if HasUnit.check(min): | ||
| num_min = min.to_base_units().magnitude |
There was a problem hiding this comment.
This can not work. HasUnit.check checks if something has an units attr, but in the following we act like the variable is a Quantity (.to_base_units)
There was a problem hiding this comment.
Might been a better IsQuantity check then, or I can duck-type it w/ the attribute.
It was to guard against this issue: #141 (comment)
| def within(self) -> Ranges | None: | ||
| return self._within | ||
|
|
||
| @within.setter |
There was a problem hiding this comment.
I guess you need this because you first make the Param then find out about its within&sofset.
Got to make sure that we take the mutable within & softset into account everywhere. Maybe limiting it with '@assert_once' might be a good idea for the moment,
There was a problem hiding this comment.
I like that idea. It won't work for the front-end because of how overrides currently work in ato.
Maybe this deserves another issue to improve safety around later? No one in their right mind should do this without being quite sure of what they're doing
Superseded by: atopile/atopile#517