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.

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

Python 3.9 incompatibility "class must be set to a class, not 'ClassReloadr' object" #13

Open
zhangyq73 opened this issue May 6, 2021 · 6 comments
Labels

Comments

@zhangyq73
Copy link

I run 01_manual_reload.py in the examples directory,error is as follows:
TypeError: class must be set to a class, not 'ClassReloadr' object
python-BaseException.
I am using python3.9 on the win10 platform,What is the cause of this error?

@hoh hoh added the bug label May 6, 2021
@hoh
Copy link
Owner

hoh commented May 6, 2021

Looks like Python 3.9 changes the way it handles classes internally.

I can reproduce the issue using Python 3.9 on Linux using Docker image python:3.9-buster. The example works fine using Python 3.8 via python:3.9-buster.

Traceback:

$ python 01_manual_reload.py 
Car on 1001 3001
Traceback (most recent call last):
  File "reloadr/examples/01_manual_reload.py", line 28, in <module>
    Car._reload()
  File "/usr/local/lib/python3.9/site-packages/reloadr.py", line 136, in _reload
    instance.__class__ = self._target
TypeError: __class__ must be set to a class, not 'ClassReloadr' object

@hoh hoh changed the title Run error:class must be set to a class, not 'ClassReloadr' object python-BaseException. Python 3.9 incompatibility "class must be set to a class, not 'ClassReloadr' object" May 6, 2021
@hoh
Copy link
Owner

hoh commented May 6, 2021

The issue comes from https://github.com/hoh/reloadr/blob/master/reloadr.py#L31 : in Python 3.9, inspect.getsource(class_object) returns the decorators as well.

Below is a temporary patch. The issue of decorators should be handled in a better way.

-    if kind == 'def':
-        # `inspect.getsource` will not return the decorators in the source of classes,
-        # but will return them in the source of functions.
-        source = source.replace("@autoreload\n", "")
+    source = source.replace("@autoreload\n", "").replace("@reloadr\n", "")

@P6rguVyrst
Copy link

@hoh If a regex approach is OK to remove all the decorators that follow @reloadr, then I can take this on me. I'd try to find time this week to work on it.

Screenshot 2023-02-08 at 08 23 42

I'd be looking to add a test for get_new_source function (I'm familiar with pytest, but maybe you prefer something else?), and maybe something like tox (maybe you have better recommendations) to test it with different python versions. Could add GHA to run the test and add new python versions to test with - to make sure future versions wouldn't break it, this function.

@hoh
Copy link
Owner

hoh commented Feb 12, 2023

Hi @P6rguVyrst ,

The regex approach looks fine. It would be wise to also include the following names as aliases of that decorator:

  • @reloadr
  • @autoreload
  • @reloadr.reloadr
  • @reloadr.autoreload

Testing using Pytest sounds wise. Regarding the matrix of Python versions, either Tox or GitHub Actions seem fine. I would prefer the change that fixes this issue and a simple test in one PR, and the addition of Tox, GitHub Actions, ... in another since that is not entirely related.

@P6rguVyrst
Copy link

P6rguVyrst commented Feb 22, 2023

It would be wise to also include the following names as aliases of that decorator

Took me a while, but I think I see what you mean.

Do you mean that the solution should not be limited to @reloadr only, but cover any one of those interfaces that are currently supported by the library.

    decorator_scope = [
        "@reloadr",
        "@autoreload",
        "@reloadr.reloadr",
        "@reloadr.autoreload",
    ]

Makes it a more interesting problem, but wouldn't mind giving a go extending my initial solution and try to come up with a more complete solution.

Technically speaking something like r"(@reloadr|@autoreload)(.*)(?=class)" should cover any functions in the reloadr module has + the from reloadr import autoreload

@raw2026
Copy link

raw2026 commented Jul 28, 2023

Has any further bug-fix progress been made ?
I had issues with python 3.10 and reloadr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants