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

feat: Improved __init__.py for skeletons and modules #137

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

phorward
Copy link
Member

This PR changes the current __init__.py-scripts to allow for module- and skeleton restructuring using folders.

  • skeletons/__init__.py imports skeletons from any sub-folders now
  • modules/__init__.py imports modules from any sub-folder, but registers them as they where defined just in modules, so there's no prefixing like before.

As the latter one "breaks" a current behavior, IMHO this was only requested for the viur-shop module and was solved differently there, as there is a Shop()-module which is imported.

This PR changes the current __init__.py-scripts to allow for module- and skeleton restructuring using folders.

- `skeletons/__init__.py` imports skeletons from any sub-folders now
- `modules/__init__.py` imports modules from any sub-folder, but registers them as they where defined just in modules, so there's no prefixing like before.

As the latter one "breaks" a current behavior, IMHO this was only requested for the viur-shop module and was solved differently there, as there is a `Shop()`-module which is imported.
@phorward phorward requested a review from sveneberth November 29, 2024 13:02
@phorward phorward added feature New feature or request Priority: Medium This issue may be useful, and needs some attention. labels Nov 29, 2024
Copy link
Member

@sveneberth sveneberth left a comment

Choose a reason for hiding this comment

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

I basically don't care what the recursive behavior is. I have the importer in my projects so that the order structure simply acts as a grouping of the modules for a better overview, but does not cause any name changes.
In the end, everyone has their own expectations as to how they would like this to work in their project or how it fits there.
I would be interested to hear @KadirBalku 's opinion, as he implemented it that way back then in #112.
Otherwise, the behavior would be okay for me.

I just think the technical implementation is a step backwards. Why are you using this old fashion os lib again, but still handling path objects? Please use the pathlib consistently. I think you can save yourself these __find_py_files completely if you simply use directory.rglob("*.py"). You would only have to do the blacklist manually.

for name, obj in inspect.getmembers(__module, inspect.isclass):
if issubclass(obj, __viur_Module) and obj.__module__ == __module.__name__:
# Import the class into the current namespace
print(name.lower(), obj)
Copy link
Member

@sveneberth sveneberth Nov 29, 2024

Choose a reason for hiding this comment

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

Please don't use print and use the logging framework.

Even if it is not established, maybe also a custom logger

log = logging.getLogger("my-project").getChild(__name__).getChild("autoloader")

Using the root logger is actually mostly a bad idea, but that's another problem in ViUR and ViUR projects ...

@sveneberth sveneberth added the waiting-for-changes Waiting for changes/rework from the author label Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request Priority: Medium This issue may be useful, and needs some attention. waiting-for-changes Waiting for changes/rework from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants