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

[MedApp] add python registry #11708

Merged
merged 1 commit into from
Oct 24, 2023
Merged

[MedApp] add python registry #11708

merged 1 commit into from
Oct 24, 2023

Conversation

philbucher
Copy link
Member

Adding the python registry lists for the MedApp

@rubenzorrilla I am not fully sure what this does, my imports in the projParams also worked without this 🤔

@@ -0,0 +1,11 @@
from typing import List

python_modelers_to_be_registered: List[str] = ["modelers.import_med_modeler.ImportMedModeler"]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
python_modelers_to_be_registered: List[str] = ["modelers.import_med_modeler.ImportMedModeler"]
python_modelers_to_be_registered: List[str] = ["import_med_modeler.ImportMedModeler"]

As it is in the modelers list it will automatically add the corresponding modeler keyword. Long story short, in the list it is only required to add python_module.ClassName.

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied it from the core where it is done like I do

"modelers.import_mdpa_modeler.ImportMDPAModeler"

I prefer to keep it consistent

Copy link
Member

Choose a reason for hiding this comment

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

The point is that in the core we have a submodule modeler coming from the corresponding subfolder within python_scripts. I didn't note that this is also the case in the MedApplication so you're right. Forget about my comment (I'd leave it as unresolved as "documentation").

@rubenzorrilla
Copy link
Member

Adding the python registry lists for the MedApp

@rubenzorrilla I am not fully sure what this does, my imports in the projParams also worked without this 🤔

Basically it will add all the Python entries in lists to the registry at the time the application is imported. Most probably you didn't notice any change as you're not using a registry-based factory.

@philbucher philbucher merged commit 0904e91 into master Oct 24, 2023
@philbucher philbucher deleted the med/python-registry branch October 24, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants