Add support for dunder methods to call their JS equivalent consistently about pyodide HOT 8 CLOSED

Can you please give an example of code you wrote, what it currently does, and what you would prefer for it to do?

from pyodide.

I have a python DateRange Class that I want to be able to check other dates membership inside the range. So I made the following dunder method in python:

 def __contains__(self, date_value: Union[datetime, date]) -> bool:
    date_value = self._convert_to_date(date_value)
    return self.lower_date <= date_value <= self.upper_date

but when I run the includes method on an instance of the object in Javascript with a JS Date object as the method's argument, I get a KeyError rather than running my python method.

What I was expecting was that it would work like using the in operator in python and run my __contains__ dunder method. I would expect basically the same with str and the toString method where my str method would run in JS and the same for every other dunder method with an equivalent JS function/method.

from pyodide.

Well toString() uses __repr__:

let a = pyodide.runPython(`
class A:
   def __repr__(self):
      return "a"
A()
`);
a.toString()

I don't recall why we picked __repr__ over __str__ but there isn't an obvious choice.

For __contains__ we seem to map it to has:

pyodide.runPython(`
class A:
   def __contains__(self, other):
      return isinstance(other, int) and other > 5
A()
`);
a.has(7); // true
a.has(4); // false
a.has({}); // false

If you register it as a Sequence then you can use __contains__ via includes:

let a = pyodide.runPython(`
from collections.abc import Sequence
@Sequence.register
class A:
   def __contains__(self, other):
      return isinstance(other, int) and other > 5
A()
`);
a.includes(7); // true

This is meant to line up with the way JavaScript objects work: there is Array.prototype.includes but Map.prototype.has and Set.prototype.has, etc. So only Sequence-like objects get an includes and everything else gets has instead.

from pyodide.

To the toString method using the __repr__ method, that just seems incorrect since there's a distinction between __repr__ and __str__ that python devs expect and sometimes rely on. Using __repr__ when doing a bare console.log makes since, but not when a dev is trying to string the object.

I don't know how you're running your pyodide code, but I'm running it through TS-Node and adding @Sequence.register to my code still errors out when using the includes method, as does your example. The has method does work though.

from pyodide.

Oh yes, the @Sequence.register example is on tip of tree, so it will be supported when we release v0.24.

Looking at the history, it seems that toString was made to do __repr__ when the project was three months old, which was before things were really discussed at all:
9579aa9

Since then I'm not sure if anyone has discussed whether it should do __repr__ or __str__. I would be open to changing it if there is a consensus in favor of __str__.

from pyodide.

Ahh that makes sense for the example. Regardless, it seems that the separation between includes and has in Javascript doesn't apply to python in any sensible way, so I'll leave it at what it is.

I've given my case for __str__, so I'll leave it at that for the maintainers to decide :)

from pyodide.

I'll open a separate issue and solicit some opinions.

from pyodide.

Okay I'm going to close this issue @Duncanr-glitch but feel free to reopen it if you think it hasn't been resolved, or open a new issue if you have further questions.

from pyodide.