Skip to content

Conversation

skirpichev
Copy link
Collaborator

a first step to solve #696

@oscarbenjamin
Copy link

I think this looks good.

Perhaps it could be for a separate PR but it looks like these methods could be simplified more along the lines of the _mpc methods:

def __eq__(s, t):
if not hasattr(t, '_mpc_'):
if isinstance(t, str):
return False
t = s.mpc_convert_lhs(t)
if t is NotImplemented:
return t
return s.real == t.real and s.imag == t.imag

Probably the _mpf methods should delegate the complex case to _mpc rather than having inline logic to handle it. Then you could have something like:

# class _mpf
def __eq__(self, other):
    # handle the common case up front
    if isinstance(other, _mpf):
        return mpf_eq(self._mpf_, other._mpf_)
    self_new, other_new = ctx.convert_pair(self, other)
    if self_new is NotImplemented:
        return NotImplemented
    # might both be _mpc so we call __eq__ rather than mpf_eq
    return self_new.__eq__(other_new)

@skirpichev
Copy link
Collaborator Author

Probably the _mpf methods should delegate the complex case to _mpc

I think it's a good idea, but I doubt it should be implemented there.

We also could, for example, just return NotImplemented for anything except mpf/int/floats in mpf methods. Such changes, however, must be carefully benchmarked.

@skirpichev skirpichev merged commit f815969 into mpmath:master May 21, 2023
@skirpichev skirpichev deleted the no-meta-in-_mpf branch May 21, 2023 04:36
@skirpichev skirpichev added this to the 1.4 milestone May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants