Using __getattr__ breaks python-fire

[borislopezaraoz]

This (admittedly hacky) metaprogramming way of implementing missing methods on the fly breaks python-fire.

Ref: https://stackoverflow.com/a/6955825/1096370

import fire

class Calculator(object):
    """A simple calculator class."""

    def double(self, number):
        return 2 * number

    def __getattr__(self, name):
        def _missing(*args, **kwargs):
            print("A missing method was called.")
            print("The object was %r, the method was %r. " % (self, name))
            print("It was called with %r and %r as arguments" % (args, kwargs))
        return _missing

if __name__ == '__main__':
    fire.Fire(Calculator)

This "ghost methods" works normally when called from other python module.
When trying to call it with fire I get this:

python -m calculator double 10

Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/users/dev/project/calculator.py", line 19, in <module>
    fire.Fire(Calculator)
  File "/users/dev/project/venv/lib/python3.7/site-packages/fire/core.py", line 127, in Fire
    component_trace = _Fire(component, args, context, name)
  File "/users/dev/project/venv/lib/python3.7/site-packages/fire/core.py", line 437, in _Fire
    component, remaining_args)
  File "/users/dev/project/venv/lib/python3.7/site-packages/fire/core.py", line 513, in _GetMember
    members = dict(inspect.getmembers(component))
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/inspect.py", line 330, in getmembers
    for base in object.__bases__:
TypeError: 'function' object is not iterable

[meshde]

TL;DR

• Error occurs in python3.x and not in python2.x
• Problem lies in getmembers() method of the inspect module, specifically line 330:
  for base in object.__bases__:
• For a Python object, __bases__ is an unknown attribute and throws AttributeErorr when trying to access as above. This exception is being handled by the inspect module.
• In this case, __getattr__() has been defined to return a default function that will be called when trying to call unknown methods. This default function is also returned when trying to access unknown attributes.
• Hence, object.__bases__ here will return a function, which is not iterable and the error is thrown.
• Solution: Before passing component to inspect.getmembers(), check whether the call to the __bases__ attribute results in anything other than tuple. If yes, assign an empty tuple to component.__bases__

The Problem

The error occurs in python3.x but not in python2.x. The reason being the way inspect.getmembers() has been implemented in the two versions.

The problem lies in the following snippet (spanning from lines 329-335) from the function in the python3.x version of the code:

try: 
	for base in object.__bases__:
		for k, v in base.__dict__.items():
			if isinstance(v, types.DynamicClassAttribute):
				names.append(k)
except AttributeError:
	pass 

The erorr occurs on line 2 above (line 330 from the actual code):
for base in object.__bases__:

This line was actually meant to get members of the parent classes or base classes of the current class. If the object variable happens to be a Python Object, as in the case of fire, this line tries to access the __base__ attribute of the object, which does not exist. So it throws an AttributeError which, as can be seen on line 6 above, is already being handled by the inspect module.

The problem in this particular case is that since the _getattr_() function has been defined, this function will also be called if you’re trying to access an unknown attribute along with being called for unknown methods. So __base__ in this case is an unknown attribute and __getattr__() returns the function you've defined when trying to access it. Functions in Python are not iterable, hence the error.

Possible Solutions

• One possible solution/hack could be to add a check to the line calling inspect.getmembers() such as the one below:

  try:
      if not isinstance(component.__bases__, tuple):
          component.__bases__ = ()
  except AttributeError:
      pass
  
  # Call to inspect.getmember() comes here

• Another solution is at the user end, where a similar check can be added to __getattr__:

  def __getattr__(self, name):
        if name == '__bases__':
            raise AttributeError
  
        def _missing(*args, **kwargs):
            print("A missing method was called.")
            print("The object was %r, the method was %r. " % (self, name))
            print("It was called with %r and %r as arguments" % (args, kwargs))
        return _missing

[meshde]

@borislopezaraoz you could try out the second solution till this is fixed.

[meshde]

@dbieber The first solution seems kind of a hack to me, but if it's cool with the maintainers of the project, I'll raise a PR to fix this.

[dbieber]

I think there may be a simpler solution involving removing the line members = dict(inspect.getmembers(component)) (L562)

We really just want to check for the existence of two members (arg and arg.replace('-', '_'), with precedence given to the former), so there's no reason we should have to get all the members to do these two checks.

@meshde wdyt?

[meshde]

Makes sense.

We could simply change lines L562 - L571 in _GetMember() from

  members = dict(inspect.getmembers(component))
  arg = args[0]
  arg_names = [
      arg,
      arg.replace('-', '_'),  # treat '-' as '_'.
  ]

  for arg_name in arg_names:
    if arg_name in members:
      return members[arg_name], [arg], args[1:]

to

  members = dir(component)
  arg = args[0]
  arg_names = [
      arg,
      arg.replace('-', '_'),  # treat '-' as '_'.
  ]

  for arg_name in arg_names:
    if arg_name in members:
      return getattr(component, arg_name), [arg], args[1:]

I don't think this change should break anything, because inspect.getmembers() defaults to pretty much the same thing when the passed parameter happens to be an object and _GetMember() is only called in the case where component is an object.

[borislopezaraoz]

Thanks @meshde, I can confirm that your second solution works on python 3.

In the end I went with a different approach: https://stackoverflow.com/a/534597/1096370
Using setattr to define the methods I need (which I know in advance) instead of catching all missing methods.
It seems cleaner and it doesn't break python-fire.

For other use cases when the "missing methods" are more dynamic (not known in advance) it would be nice to have that fix in place.

[Datamance]

This still breaks in 0.2.1 (the latest version on pypi) - for my use case, I don't know which method is being called in advance and I'm lazy-loading modules accordingly, a la:

    def __getattr__(self, handle: str) -> AbstractSubSystem:
        """Hook from the command line."""
        target: ImportTarget = self._APP_DICT[handle]  # "handle" ends up being "__bases__" among other things

        subsystem: Type[AbstractSubSystem] = getattr(
            import_module(target.package), target.callable
        )

it looks like line 635 in _GetMember is breaking:

    members = dict(inspect.getmembers(component))

which tells me that the change that you guys talked about here never made it into PyPI.

@meshde @dbieber any recommendations on what to do here? Is there a new release coming soon? Or should I try to find another workaround (since raising AttributeError doesn't actually solve the problem)?

[meshde]

@Datamance I have raised a PR to fix this, I seem to have missed doing so earlier.

In the meantime you could use the hack mentioned in the comments.

def __getattr__(self, name):
      if name == '__bases__':
          raise AttributeError

      def _missing(*args, **kwargs):
          print("A missing method was called.")
          print("The object was %r, the method was %r. " % (self, name))
          print("It was called with %r and %r as arguments" % (args, kwargs))
      return _missing

You mention

since raising AttributeError doesn't actually solve the problem

Are you facing issues with this hack?

[Datamance]

@meshde correct - even with that check, it'll break on "_fields", and then if I check for that, "FIRE_METADATA"

[Datamance]

Actually, before it breaks on _fields I do get some extra stack trace information:

Traceback (most recent call last):
  File ".../3.8.0/lib/python3.8/inspect.py", line 1123, in getfullargspec
    sig = _signature_from_callable(func,
  File ".../3.8.0/lib/python3.8/inspect.py", line 2216, in _signature_from_callable
    raise TypeError('{!r} is not a callable object'.format(obj))
TypeError: <orchestrator.core.Application object at 0x10157b9a0> is not a callable object

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File ".../.local/share/virtualenvs/lookit-orchestrator/lib/python3.8/site-packages/fire/inspectutils.py", line 111, in GetFullArgSpec
    kwonlyargs, kwonlydefaults, annotations) = inspect.getfullargspec(fn)  # pylint: disable=deprecated-method,no-member
  File "....pyenv/versions/3.8.0/lib/python3.8/inspect.py", line 1132, in getfullargspec
    raise TypeError('unsupported callable') from ex
TypeError: unsupported callable

[Datamance]

Now that I'm looking closer, I'm looking at your proposed fix @meshde and realizing it won't work for my use case:

  members = dir(component)
  arg = args[0]
  arg_names = [
      arg,
      arg.replace('-', '_'),  # treat '-' as '_'.
  ]

  for arg_name in arg_names:
    if arg_name in members:  #  <<<<<< ------- <<<<<< [THIS LINE RIGHT HERE]
      return getattr(component, arg_name), [arg], args[1:]

The check I've pointed to above precludes the use of getattr as a mechanism for truly dynamic, on-the-fly generation of values not in the object or class dict - members = dir(component) just gets predefined object properties as before.

If I have __getattr__ dynamically generating new objects on the fly (as one might do with lazy loading, see below):

    def __getattr__(self, handle: str) -> AbstractSubSystem:
        """Hook from the command line."""

        target: ImportTarget = self._APP_DICT[handle]

        subsystem: Type[AbstractSubSystem] = getattr(
            import_module(target.package), target.callable
        )

        self._subsystem = subsystem.from_config(self._config)
        return self._subsystem

then those objects will definitionally never appear in members.

This brings my next question to bear - why check for membership at all - why not just use getattr as it's meant to be ("ask forgiveness not permission")?

for arg_name in arg_names:
    sub_component = getattr(component, arg_name, None)
    if sub_component:
        return sub_component, arg, args[1:]