-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
base: master
Are you sure you want to change the base?
Conversation
This PR successfully silences the error so autoclass works with Python 3.9. I have not investigated the code in depth to be cable to guarantee it as a full solution.
@@ -257,9 +257,6 @@ def execute_autodict_on_class(cls, # type: Type[T] | |||
'__contains__', | |||
'get', | |||
'items', |
There was a problem hiding this comment.
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)
``
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Thanks a lot @erocarrera ! Sorry for not spotting your PR sooner. As commented above, it seems that you modified a try/except branch that was dedicated to a special case of python 2... So the issue is rather to understand why the code on python 3.8+ enters this branch at all, in other words to understand why
fails in the first place. |
(replying to #44 (comment) ) Perfect @erocarrera thanks for the feedback ! Indeed moving to pyfields when possible is probably a better choice. I did not yet found time to fix this autoclass issue, sorry :( we'll see if many users are interested or if they all move away to pyfields, attrs, dataclasses and the like |
This PR successfully silences the error so autoclass works with Python 3.9. I have not investigated the code in depth to be cable to guarantee it as a full solution.