Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to fix issue #43 #44

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Attempt to fix issue #43 #44

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions autoclass/autodict_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def autodict(include=None, # type: Union[str, Tuple[str]]
* it adds a static from_dict method to build objects from dicts (only if only_constructor_args=True)
* it overrides eq method if not already implemented
* it overrides str and repr method if not already implemented

Parameters allow to customize the list of fields that will be visible.

:param include: a tuple of explicit attribute names to include (None means all)
Expand Down Expand Up @@ -257,9 +257,6 @@ def execute_autodict_on_class(cls, # type: Type[T]
'__contains__',
'get',
'items',
Copy link
Owner

@smarie smarie May 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removal makes the python 2 test fail since this if/else statement is specifically for python 2. We should not modify this branch but rather try to understand why the main path fails lines 234-239

try:
            cls.__bases__ = new_bases
        except TypeError:
            try:
                # maybe a metaclass issue, we can try this
cls.__bases__ = with_metaclass(type(cls), *new_bases)
``

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing me in the right direction.
Under Python3.9, the code: cls.__bases__ = new_bases throws TypeError: __bases__ assignment: 'Mapping' deallocator differs from 'object' which I'm guessing might be the exception you're trying to catch.

The cls.__bases__ = with_metaclass(type(cls), *new_bases) throws TypeError: can only assign tuple to Experiment.__bases__, not metaclass (Experiment is the name of my class where I'm using autoclass)

I naively tried to make it a tuple as cls.__bases__ = (with_metaclass(type(cls), *new_bases),) and I get the exception TypeError: __bases__ assignment: 'temporary_class' deallocator differs from 'object'

I don't know what to try next, but maybe this gives you some information?

Copy link
Owner

@smarie smarie May 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for providing this ! Well this does not look good. I understand why you ended up fixing the last part of the try/except.

Not sure that I can fix this very soon. I'll have a look in the upcoming weeks.

If you wish to have a stab at it in between, you can try to

  • revert the changes you did,
  • create a boolean version flag (PY38 = sys.version_info >= (3, 8))
  • use that flag in the last "except" (just where you did your first mod) to skip the method names that are not relevant anymore, and to not use .im_func.

Also note that a more recent lib that I consider better than autoclass for my personal use is https://smarie.github.io/python-pyfields/ . It also provides an @autoclass decorator. Instead of fiddling with the class bases, this decorator adds to_dict and from_dict methods.

Still of course I try to maintain both when possible

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Sylvain, I've moved to pyfields as you suggested, that solved the issue for me. I will not grapple with trying to address the issue in autoclass. Feel free to close the PR if you want.

'iteritems',
'iterkeys',
'itervalues',
'keys',
'values']
# from inspect import getmembers
Expand All @@ -268,8 +265,8 @@ def execute_autodict_on_class(cls, # type: Type[T]
# meths = getmembers(Mapping.get(), predicate=is_useful)
# for name, func in meths:
for name in names:
# bind method to this class too (we access 'im_func' to get the original method)
setattr(cls, name, getattr(Mapping, name).im_func)
# bind method to this class too
setattr(cls, name, getattr(Mapping, name))
erocarrera marked this conversation as resolved.
Show resolved Hide resolved

# 3. add the static class method to build objects from a dict
# if only_constructor_args:
Expand Down Expand Up @@ -400,7 +397,7 @@ def autodict_override_decorate(func # type: Callable


autodict_override = autodict_override_decorate
"""A decorator to indicate an overridden dictionary method. In this case autodict will not override it and will not
"""A decorator to indicate an overridden dictionary method. In this case autodict will not override it and will not
generate a warning"""


Expand Down