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

[ShapeOpt] add missing init files #9507

Merged
merged 1 commit into from
Jan 10, 2022
Merged

Conversation

armingeiser
Copy link
Member

📝 Description
The subfolders of the python scripts folder in the ShapeOptApp had no __init__.py files.

I don't know why it works without but its good practice to have those files in each folder.

🆕 Changelog

  • Added __init__.py to all submodules in ShapeOptimizationApplication

@armingeiser armingeiser requested a review from a team as a code owner January 3, 2022 10:11
@armingeiser armingeiser changed the title [Shape/Opt] add missing init files [ShapeOpt] add missing init files Jan 3, 2022
@miguelmaso
Copy link
Contributor

I don't know why it works without but its good practice to have those files in each folder.

it was introduced in #3217

@philbucher
Copy link
Member

I don't know why it works without but its good practice to have those files in each folder.

it was introduced in #3217

actually the solution from #3217 was only an intermediate solution that was removed when we switched to the new installation mechanism (#5825).
Basically #3217 enabled us to use the python-scripts properly (i.e. from Kratos.AppName import py_script.py instead of import py_script.py) This was a prerequisite for #5825

I am not sure why it works without the __init__.py. The CoSimApp has many subfolders but so far it was always working without

@roigcarlo do you know better?

@sunethwarna
Copy link
Member

Isn't it because of the following that we can use python imports eventhough we don't have __init__.py files?

https://docs.python.org/3/whatsnew/3.3.html#pep-420-implicit-namespace-packages

@armingeiser
Copy link
Member Author

Hm, maybe it works because of the namespace package thing as @sunethwarna says.

But from what i understand we are not actually using namespace packages, but all our apps should rather be "standard" packages, thus the `init.py``files should be there.

Copy link
Member

@roigcarlo roigcarlo left a comment

Choose a reason for hiding this comment

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

Exactly as @sunethwarna says. As we had support for 2.x when we make the change to use standard python modules it wouldn't have worked without. In any case I still think is a good idea to have the __init__.py file, among other reasons because not all our folders are sub-modules and this is a clean way to differentiate them.

@armingeiser armingeiser merged commit 55f9dbc into master Jan 10, 2022
@armingeiser armingeiser deleted the ShapeOpt/add-missing-init branch January 10, 2022 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants