Codecov Report

Attention: Patch coverage is 72.72727% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 92.70%. Comparing base (5bd4ae5) to head (57a74ab).

@@            Coverage Diff             @@
##             main      #38      +/-   ##
==========================================
- Coverage   93.51%   92.70%   -0.82%     
==========================================
  Files          14       14              
  Lines         632      644      +12     
==========================================
+ Hits          591      597       +6     
- Misses         41       47       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@blegat is the mutability trait even needed now?

Since we are calling operate! explicitly, it's not necessary to implement mutability indeed. However, the user could call operate!! in which case it's useful but then mutability should return IsNotMutable if the storage is a tuple. About the subtyping <:AbstractMutable, it's very useful so that we get fast matrix-vector multiplications etc... from MutableArithmetics/src/dispatch.jl

EDIT: test will fail due to broken test now fixed.

🎉 I turned them to not broken

very well, but I'm not sure that this is the way forward -- adding this canonical method is rather a hack. Much cleaner imo would be to return to calling operate!! where appropriate, which utilizes the mutability trait.

Why would we need operate! only for canonical, I don't quite get where this is called and why we need to canonicalize if we didn't do any operate! before

Then maybe we think of canonical in a very different way. For me objects manipulated (algebra elements) do not need to be in their canonical form which makes all the operations cheaper (e.g. lazy). Only when canonical form is actually needed (e.g. in ==) the objects are brought to it.
On the other hand canonicalisation of an object in canonical form should be as cheap as possible (possibly as fast as comparison of a Bool).