sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.42k stars 478 forks source link

Disable fast_getattr to allow usage of memoryview? #38839

Open user202729 opened 1 week ago

user202729 commented 1 week ago

Problem Description

Currently, we cannot use memoryview in SageMath because of what appears to be a cython bug: https://github.com/cython/cython/issues/6442

As recommended in cython documentation, typed memoryview is preferred to the numpy.ndarray[...] syntax.

Proposed Solution

We disable fast_getattr as suggested in the cython bug linked above.

Side note, the commit that introduces fast_getattr is 7f33b867439e414cceb568547b2c21b216b33244 which is #10493 --- however I can't find any discussion on why the fast_getattr is added?

Alternatives Considered

-

Additional Information

No response

Is there an existing issue for this?

da-woods commented 1 week ago

My working theory is that the directive isn't actually doing much - it prevents Cython from creating a __getattr__ C method in the class. However that just affects what happens if you do Cls.__getattr__ or Cls().__getattr__. I don't think it has any impact at all on the speed of attribute lookup.

So it'd be good to know if I'm missing something and there's actually a good reason for this.