Question about usage of the new `_names_cache` in another library

[ikrommyd]

Hello,

In this module of Awkward Array: https://github.com/scikit-hep/awkward/blob/main/src/awkward/_connect/numexpr.py
we implement "bindings" of awkward arrays to numexpr so that numexpr can evaluate expressions between awkward arrays.

In the latest numexpr release (2.11), there is an issue with the following lines: https://github.com/scikit-hep/awkward/blob/8e2d4a85420e89617ccf5be0edef841d3b2d57bf/src/awkward/_connect/numexpr.py#L82-L86

The problem is evident because since commit 33ee71b, numexpr.necompiler._names_cache is no longer a CacheDict but a threading.local() and therefore the line expr_key not in numexpr.necompiler._names_cache raises TypeError: argument of type '_thread._local' is not iterable naturally.

I'm opening this issue to ask for any suggestions to minimally overcome this since many things changed internally in the latest numexpr. Is it to just initialize our own CacheDicts inside this module?

[FrancescAlted]

Hello. As you probably know, you should not use private variables in (like numexpr.necompiler._names_cache) in your wrappers. Having said that, maybe @andfoy has some suggestion to your issue.

[ikrommyd]

Yeah so I did this: scikit-hep/awkward#3533 which makes the tests pass but I'm not sure if I'm missing something. The wrapper just implements an evaluate and a re_evaluate function like the raw numexpr compiler does so I tried to mimic a similar caching there as well.

You're arguing that we should not use the private caches at all in the wrappers.

[ikrommyd]

The difference is that in numexpr 2.11, you are initializing those caches inside the validate function so they are not directly accessible at the module level. So I tried to mimic that by initializing them inside the evaluate function that awkward array implements.

[andfoy]

As @FrancescAlted mentioned, it is not recommended to modify private attributes from an external library, since any change or removal of these might incur in code breakages downstream. In this case in particular, you can initialize the relevant threading.local() arguments by doing the following stanza:

import threading

my_local = threading.local()

if not hasattr(my_local, 'attr'):
      my_local.attr = some_value

[ikrommyd]

So your recommendation is to use our own caches completely in a similar way the numexpr compiler does but not touch/use the ones the numexpr compiler uses at all? But shouldn't numexpr wrapper use the same caches as numexpr? In the end, numexpr is used internally to evaluate numpy expressions.

Edit: Yeah using different dictionaries doesn't work for re-evaluate because the dictionaries do not get filled by numexpr in this case. It seems like we have to use the same dictionaries. We were using the same _numexpr_last that numexpr uses before it was turned into a threading.local object. _numexpr_last should be filled by numexpr, not our code.

[andfoy]

@ikrommyd sorry for the confusion, what I meant is to initialize the private numexpr dictionaries like numexpr does in case you are depending on them, that shouldn't affect numexpr initialization, which is conditional:

numexpr/numexpr/necompiler.py

Line 860 in bbf4672

if not hasattr(_names_cache, 'c'):

[ikrommyd]

Yeah I added such an initialization inside the evaluate and re-evaluate functions that we implement. I’m basically making it so that we initialize the private dictionaries for numexpr before numexpr does in the same way. Tests pass, indeed it’s bad that we depend on internal code but that’s how’s it’s always been for these.