Discuss: use slots for all classes in leoNodes.py

[Edward K. Ream]

This is PR #2003 and issue #2004.

Unless I hear compelling arguments against, I plan to "freeze" all the classes in leoNodes.py (Position, VNode, NodeIndices and PosList classes) by defining a __slot__ attribute for each class.  It is straightforward to do this.  See PR #2003.

Rationale

mypy recently discovered a bug arising from a misspelling of an ivar of the VNode class. This freaked me out. Without slots, pylint does not (and can not) detect such errors because there is no of knowing whether the code actually intended to inject a new ivar.

Without slots, Leo's most important data classes can neither be checked nor protected properly!

Summary

Imo, using slots in leoNodes.py is overdue. I know of no drawbacks to doing so.

Slots have at least these advantages:

- Slots help pylint and mypy check code.
- Slots reduce the memory footprint of positions and vnodes.
- Slots prevent unwanted alterations of Leo's most important classes.

The work is in the ekr-slots branch.  I'll wait for a few days for your comments.

Edward

[tbp1...@gmail.com]

Would you give an example of how these slots would be used, please?

[Edward K. Ream]

On Thu, Jun 24, 2021 at 2:12 PM tbp1...@gmail.com <tbp1...@gmail.com> wrote:

Would you give an example of how these slots would be used, please?

Good question. Typical Leo users will not notice any change whatsoever.

The only sense in which slots are "used" is that slots prevent scripts (or any part of Leo itself) from "injecting" new ivars into the classes. This restriction allows pylint  and mypy to detect misspellings and other serious errors.

The VNode class already has an extension mechanism, namely v.unknownAttributes. That should be plenty good enough.

The Position class should never ever need any kind of extension, so adding slots is protection, pure and simple.

Edward

[Edward K. Ream]

On Thursday, June 24, 2021 at 3:09:29 PM UTC-5 Edward K. Ream wrote:

> The Position class should never ever need any kind of extension, so adding slots is protection, pure and simple.

Slots also reduce the stress on python's GC (garbage collector) and slots also reduce access times to ivars. However, Leo already is careful not to allocate positions needlessly, so the resulting performance improvements are of little practical importance.

Edward

[Edward K. Ream]

On Thursday, June 24, 2021 at 12:48:33 PM UTC-5 Edward K. Ream wrote:

This is PR #2003 and issue #2004.

Unless I hear compelling arguments against, I plan to "freeze" all the classes in leoNodes.py (Position, VNode, NodeIndices and PosList classes) by defining a __slot__ attribute for each class.  It is straightforward to do this.  See PR #2003.

The new code has now been merged into devel.

In the extremely unlikely event that slots ever cause problem, removing them from a class is simply a matter of eliminating the class's __slot__ statement.

Edward

[Edward K. Ream]

On Thursday, June 24, 2021 at 12:48:33 PM UTC-5 Edward K. Ream wrote:

This is PR #2003 and issue #2004.

Unless I hear compelling arguments against, I plan to "freeze" all the classes in leoNodes.py (Position, VNode, NodeIndices and PosList classes) by defining a __slot__ attribute for each class.

Adding slots prevents plugins from directly injecting attributes into Position objects.

#2015 suggests adding p.pluginDict as a workaround.

All comments welcome.

Edward

[Alexey Tikhonov]

Leo (devel) starts regularly terminating when I use quicksearch window. It says:

Traceback (most recent call last):

  File "C:\Users\atikhonov2\Workarea\leo-editor\repo\leo\plugins\quicksearch.py", line 333, in returnPressed

    self.scon.doSearch(t)

  File "C:\Users\atikhonov2\Workarea\leo-editor\repo\leo\plugins\quicksearch.py", line 598, in doSearch

    bm = self.find_b(bpat, bNodes, flags)

  File "C:\Users\atikhonov2\Workarea\leo-editor\repo\leo\plugins\quicksearch.py", line 686, in find_b

    pc.matchiter = t2

AttributeError: 'Position' object has no attribute 'matchiter'

Do you think it can be related to that change?

Alexey

[Edward K. Ream]

On Wed, Jun 30, 2021 at 3:38 AM Alexey Tikhonov <tickli...@gmail.com> wrote:

Leo (devel) starts regularly terminating when I use quicksearch window. It says:

...

    pc.matchiter = t2

AttributeError: 'Position' object has no attribute 'matchiter'

Do you think it can be related to that change?

Yes. Almost for sure.

I have just created #2022. I'll fix it immediately.

Edward