There could be a special instance of p that plays the role of null, in that 
it returns None when tested.  Call it p_none.  mypy would recognize it as 
the same type as type(p) so it would be type safe, and p.__bool__ would 
return False when tested if p were p_none.  There would only need to be one 
p_none instance.

I'm not sure how many different places in the codebase that p is set to 
None.  If it's only a few, changing them all to set p to p_none instead 
shouldn't be hard or dangerous. 

On Wednesday, March 3, 2021 at 9:12:16 AM UTC-5 Edward K. Ream wrote:

> This Engineering Notebook post discusses a mypy complaint that probably 
> should never be eliminated. Feel free to ignore, unless you are Félix :-)
>
> Two Position methods, p.moveToNthChild and p.moveToParent, set p.v = None 
> if there is no such position.  This is a fundamental feature of *all* of 
> the p.moveTo* methods! All these methods change p *in place*. They do 
> *not* return a new position.
>
> Here is p.moveToNthChild:
>
> def moveToNthChild(self, n: int):
>     p = self
>     if p.v and len(p.v.children) > n:
>         p.stack.append((p.v, p._childIndex))
>         p.v = p.v.children[n]
>         p._childIndex = n
>     else:
>         # mypy rightly doesn't like setting p.v to None.
>         # Leo's code must use the test `if p:` as appropriate.
>         p.v = None  # type: ignore
>     return p
>
> As noted in the comment, the *only* valid way to test whether any of the 
> p.moveTo methods yields a valid position is to test `if p:` or `if not 
> p:`.  The p.__bool__ special method enforces this convention:
>
> def __bool__(self):
>     """
>     Return True if a position is valid.
>     
>     The tests 'if p' or 'if not p' are the _only_ correct ways
>     to test whether a position p is valid.
>     
>     Tests like 'if p is None' or 'if p is not None' will not
>     work properly.
>     """
>     return self.v is not None
>
> To repeat, mypy *rightly* complains that assigning None to p.v is a type 
> error. Changing the declaration of p.v in p.__init__ from:
>
> self.v: "VNode" = v
>
> to
>
> self.v: Optional["VNode"] = v
>
> creates a cascade of (valid!) mypy complaints. Rather than attempt a 
> (possibly heroic) fix, I think it best to leave the two `type: ignore` 
> statements in place, with comments explaining their significance.
>
> *Summary*
>
> Setting p.v to None *is a real type error.*  Imo, it would not be helpful 
> to pretend otherwise. Setting p.v to None works only because:
>
> 1. p.__bool__ tests p.v.
>
> 2. Leonine scripts must test `if p:` or `if not p:` after calling 
> p.moveTo* methods.
>
> There is *no way* to change this special case now. It affects all Leonine 
> scripts. Any anyway, I would not likely change this coding convention even 
> if I could.
>
> All comments welcome. I would especially like to know how leojs handles 
> this issue.
>
> Edward
>

-- 
You received this message because you are subscribed to the Google Groups 
"leo-editor" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/leo-editor/74ad5737-c13f-4288-80db-d1333a3ff20cn%40googlegroups.com.

Reply via email to