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

Current structure of code leads to circular dependencies or classes as modules #352

Closed
devclinton opened this issue Sep 11, 2019 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@devclinton
Copy link
Member

Because of the way we important files into init.py and the fact are class files are the same name of the class, we end up with either a circular import or modules confused as classes. We should either

  • change class file names to snake case
  • Do away with imports into init.py

@ckirkman-IDM @ZDu-IDM @braybaud , what is your preferred option?

@ckirkman-IDM
Copy link
Contributor

I can live with either choice, but my personal preference would be to probably empty out the init files and force the use of the full names e.g.
from abc.def.PythonSimulation import PythonSimulation

Primarily because keeping the class loads in the init files means, for consistency, we will have to add every future py file as an import in the appropriate init (or end up with some classes loaded, some not, with different pathing required). It's one more thing to remember to do.

@ZDu-IDM
Copy link
Collaborator

ZDu-IDM commented Sep 11, 2019

To make it simple, I prefer to use the full names (both actually are fine). Currently I removed Platform import from init.py and use the following instead wherever needed:

from idmtools.core.PlatformFactory import Platform

@devclinton
Copy link
Member Author

I still think we should snake_case the filenames as well so we can have a unique reference name for both the class and file.. ie platform_simulation and PythonSimulation

@devclinton
Copy link
Member Author

From https://www.python.org/dev/peps/pep-0008/#package-and-module-names

Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged.

Classes:

Class names should normally use the CapWords convention.

@JSchripsema-IDM
Copy link
Collaborator

Just chiming in to note that the current naming structure causes the class name to be repeated in the signature for API docs built with Sphinx too. You can see this in the DTK-Tools docs here: https://institutefordiseasemodeling.github.io/Documentation/dtk-tools/dtk/dtk.utils.Campaign.utils.html#module-dtk.utils.Campaign.utils.CampaignManager

@devclinton devclinton self-assigned this Sep 13, 2019
@devclinton devclinton added this to the 0.2.1 milestone Sep 13, 2019
@devclinton devclinton added the bug Something isn't working label May 5, 2020
shchen-idmod pushed a commit to shchen-idmod/idmtools-1 that referenced this issue Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants