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

Use of relative imports or not ? #4

Open
davidwaroquiers opened this issue Oct 14, 2020 · 2 comments
Open

Use of relative imports or not ? #4

davidwaroquiers opened this issue Oct 14, 2020 · 2 comments
Labels
optional Something we don't know yet if we will do it

Comments

@davidwaroquiers
Copy link
Collaborator

In the end, we should decide whether we use relative imports in the relevant places (e.g. when we are in atomate/abinit/..., just do something like from .fireworks import scf_fw).

Pros: more concise
Cons: less clear => sometimes difficult to know from where something is imported

@davidwaroquiers davidwaroquiers added the optional Something we don't know yet if we will do it label Oct 14, 2020
@ml-evs
Copy link
Collaborator

ml-evs commented Oct 14, 2020

I think this is best handled on a case-by-case basis depending on the submodule that is doing the importing.

If it is library code that is going to be used as-is, e.g. core classes/data, then I think relative imports are preferable if the import is from within the same module. I personally prefer to stay away from e.g. from ..fireworks import scf_fw.

If it is code that could be used as a template, e.g. a user might want to create their own custom SCF workflow using the existing one as a starting point, then absolute imports are better as the code can just be reused directly. I'm not sure how common this use-case is for atomate.

@utf
Copy link
Collaborator

utf commented Oct 16, 2020

I would suggest going for absolute imports. I can imagine we will have a number of similarly named modules for vasp/abinit/other codes. I.e.,

atomate.common.powerups...
atomate.vasp.powerups...
atomate.abinit.powerups...

Having the full path will remove any ambiguity and avoid code such as:

from .powerups import xyz
from ..common.powerups import xyz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optional Something we don't know yet if we will do it
Projects
None yet
Development

No branches or pull requests

3 participants