pwwang/python-varname

Exception raised when assigning to subscript even using raise_exc=False

GeorgeScrivener opened this issue · 4 comments

An exception is raised when attempting to assign in to a subscript, even when setting raise_exc=False

Here's a minimal reproduction of the problem:

def my_func():
    return varname(raise_exc=False)

x[0] = my_func()

which raises: Can only get name of a variable or attribute, not Subscript

This appears to be being caused here:

names = node_name(target)

as raise_exc doesn't propagate to node_name.

Adding a raise_exc argument to node_name would work, though I'm not familiar enough with the codebase to know if there's a better way?

raise_exc is (at least originally, I think) intended for suppressing a different kind of error, e.g. not being able to retrieve source code. In this case the problem is improper usage, as it's not obvious what the name of x[0] (or subscripts in general) should be.

Similarly, you will get an error with x, y = my_func() saying Expect a single variable on left-hand side, got 2.

On the other hand, my_func() without any assignment will respect raise_exc, and that's also invalid usage, so the library is not being consistent.

Seems like there should be a way to distinguish between "Something is wrong and retrieving the name is not working, you will have to find a workaround" and "This function is being used incorrectly, tell your user to assign it to a variable".

Okay that makes sense thanks, though it would be good to update the docs to make that clearer as I'd expect it to be functionally equivalently to wrapping the call in try/except, which is my current workaround.

The example I gave was improper use but I think it still represents a valid use of the library.
My use case is for writing a library that has an API somewhat similar to tensorflow and I'd like to automatically name variables in the same way e.g.

# The tensor automatically gets the name 'conv1' which can be used later by external tooling
conv1 = Conv2D(32, 3, activation='relu') 

where the call to varname is somewhere inside Conv2D.
It would be perfectly valid for the library user to store Conv2D(...) in a list in which case the tensor would be unnamed.

@alexmojaki You guess is correct. VarnameRetrievingError initially does not distinguish those two situations: improper use and inability to retrieve node.

We may need two different Exception classes to do so.

@GeorgeScrivener Another easy solution you could try is:

from varname.helpers import register
Conv2d = register(Conv2d)

conv1 = Conv2D(32, 3, activation='relu') 

# isinstance(conv1, Conv2d)
# conv1.__varname__ == 'conv1'

Just pay attention that this solution is not thread-safe.

An ImproperUseError is added to distinguish improper use of varname() from ast node retrieving failure, at v0.6.4.

For now, ImproperUseError is a subclass of VarnameRetrievingError, so that you don't have to modify your code when you try to use VarnameRetrievingError to catch the error:

try:
  x[0] = my_func()
except VarnameRetrievingError:
  ...

But in the future, ImproperUseError will become independent of VarnameRetrievingError.

Keeping this issue open until ImproperUseError becomes independent.