python file names colide with global scope variables/modules

[EmilStenstrom]

Discussed in #430

^{Originally posted by franciscobmacedo April 11, 2024}

The problem

If a component directory has python files in it that are named after some global scope variable name or module of another library, it overrides that variable. This causes a big problem: I can't name my python files after any global scope variable in my project.

Example

I have a simple component called "select". the file structure is something like this:

 sampleproject/
    ├── components/             
    │   └── select/           
    │       ├── select.py     # <- the problem
    │       └── template.html 
    ├── sampleproject/
    ├── manage.py
    └── requirements.txt

However, I also use celery, which uses the select I/O module from the standard library. When launching a celery task, it was causing this error:

AttributeError: module 'select' has no attribute 'select'

Mostly because in my component select.py file I don't really have anything named select. For testing purposes I added an empty "django.py" file in that directory, and sure enough the same problem happens:

ImportError: cannot import name 'VERSION' from 'django' (/path/to/project/components/select/django.py)

PS: The variable names inside the python file and the registered component name don't cause any issues. It's only the python file(s) name(s) inside a component directory.

Current solution

If I set the COMPONENTS settings variable to disable autodiscover, this is all fixed:

COMPONENTS = {
    "libraries": [
        "components.select",
    ],
     "autodiscover": False,
}

Proposed ideas

I understand the problem lies somewhere in the import_file function, where each component is registered as an independent module and therefore overriding already existing ones.

I'm not adding this problem as an issue because there's a solution for it. However, I believe this is a limitation: I think one should be able to name a component python file "select.py" and not worry about repercussions. Or, at least, maybe this could be more explicit in the README.md?

I don't want by all means complain, I love this project, so thank you very much for it!! Just thought this can be a problem for others as well.

Thoughts?

[EmilStenstrom]

@franciscobmacedo I agree that this is not optimal behaviour. There's probably an easy fix that entails putting the components in the a component specific scope instead of the global one. I'll happily accept patches that changes that.

[JuroOravec]

Just a thought since I was dealing with imports yesterday (but for the template/media relative paths), and this seems like a similar issue.

If my project structure is

 sampleproject/
    ├── components/             
    │   └── select/           
    │       ├── select.py     # <- the problem
    │       └── template.html 
    ├── sampleproject/
    ├── manage.py
    └── requirements.txt

then I would expect that the Python import path to for select.py would be components.select.select. Hence IMO it should NOT collide with libraries like select.

I don't think I've used the autodiscover before. So just to be sure, @EmilStenstrom are the autodiscovered files imported and used elsewhere in the code? Or is the whole purpose just to load the component files, so that the component class is registered with @component.register("mycomponent")?

If it's the latter, then we could possibly skip this assignment since it has has no purpose for Django components:

sys.modules[spec.name] = module

Or alternatively we could try to resolve the autoimported files to a correct module name. E.g. assuming that I ran Django from within the sampleproject dir, then select.py would have relative path ./components/select/select.py, so we would turn that into components.select.select

I'll try to give this a try this weekend.

Btw @franciscobmacedo great work getting to the root of this issue!

[franciscobmacedo]

Thanks! It took me a while to find out what was the problem - I thought it was something with my python version 😂

[EmilStenstrom]

@JuroOravec The only purpose of autodiscover is not having to import the component classes manually (import them in your __init__.py files or whatever). So simply skipping putting them into the global scope sounds like a good idea.

[franciscobmacedo]

the modules lookup is used in some other places, specifically:

• the __init_subclass__ method of the Component, where the component hash is created
• the _resolve_component_relative_files, where the file path to the component is extracted

I believe the module path should be like you said @JuroOravec but I'm not sure how.

I made an initial PR, but it's still not working properly

[JuroOravec]

Hi, I made a separate MR for the fix, as I was just tinkering a lot on my fork: #435

So, simply removing that line of code didn't work, because then the relative paths for templates feature didn't work (as @franciscobmacedo mentioned, in _resolve_component_relative_files). But what I found is that we can use importlib.import_module, and then everything works fine.

@franciscobmacedo in that case we can close #434