-
-
Notifications
You must be signed in to change notification settings - Fork 75
Description
Description
Hello, this is a follow-up issue of #405, it is not a bug but rather an opinionated proposal to simplify a part of the API.
Currently, fd.dx() handles None and empty tuples, but rejects other empty collections like list and set. This is not intuitive and confusing. It can lead to edge cases. Taking these types and their conversions into account add complications to the user's codebase.
fd.dx() ✅ # Equivalent to Measure('cell', subdomain_id='everywhere')
fd.dx(None) ✅ # Same as above
fd.dx([]) ❌ # Raises an error
fd.dx(()) ❓ # Accepted, but becomes Measure('cell', subdomain_id=()), not 'everywhere'
fd.dx(set()) ❌ # Raises an errorProposed solution
The simplest and the most intuitive way would be to act same as Python and its ecosystem does. This makes the API flexible and predictable, avoiding surprises for users.
Python
- Supports built-in collections consistently.
>>> sum([])
0
>>> sum(())
0
>>> sum(set())
0- It fails whem there is nothing to sum over.
>>> sum()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: sum() takes at least 1 positional argument (0 given)Numpy
- Supports built-in collections consistently.
>>> np.sum([])
np.float64(0.0)
>>> np.sum(())
np.float64(0.0)
>>> np.sum(set())
set()- It fails whem there is nothing to sum over.
>>> np.sum()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: sum() missing 1 required positional argument: 'a'I think this API change could make Firedrake/UFL more intuitive and user-friendly. It might also simplify the underlying implementation by reducing special cases, such as the differences between empty tuples and no-argument scenarios:
Line 228 in 760acc4
| def __call__( |
I’d appreciate hearing your thoughts. Does this approach seem reasonable to you?
Thank you in advance for your feedback