Wildcard import

[rijobro]

Description

Avoid import *'s. Add to runtests.sh for future additions.

Status

Ready

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Integration tests passed locally by running ./runtests.sh --codeformat --coverage.
• Quick tests passed locally by running ./runtests.sh --quick.
• Documentation updated, tested make html command in the docs/ folder.

[wyli]

this is a good point, is it possible to achieve the same by not ignoring F405 flake8 and modifyiing some config?

MONAI/setup.cfg

Line 64 in a74fef9

E203,E305,E402,E501,E721,E741,F403,F405,F821,F841,F999,W503,W504,C408,E302,W291,E303,

[rijobro]

Yup, looks good. What do you think I should I do for import * in __init__.py files?

[wyli]

Yup, looks good. What do you think I should I do for import * in __init__.py files?

this tool suggests to set __all__ https://lgtm.com/projects/g/Project-MONAI/MONAI/alerts/?mode=tree&ruleFocus=3980091 what do you think?

[rijobro]

import * is fine when __all__ is defined, but I think readability is reduced since you don't necessarily know where something was imported from. Personally, I would prefer explicitly importing individual modules.

But I'm not sure if import * is OK inside of an __init__.py file...

[wyli]

import * is fine when __all__ is defined, but I think readability is reduced since you don't necessarily know where something was imported from. Personally, I would prefer explicitly importing individual modules.

But I'm not sure if import * is OK inside of an __init__.py file...

sure I'm fine with either. I think both are valid for __init__.py. @ericspod @Nic-Ma comments?
(https://lgtm.com/rules/3980091/)

[ericspod]

Wildcard imports in __init__.py files is standard if __all__ is defined in the module being imported. Wildcard imports elsewhere are discouraged by flake8 and other sources. Somehow you want to export from a module the things you want to be part of the public interface, so filtering is done in the __init__.py file with selective imports from submodules or with wildcards and __all__ used in submodules to define what's public. From what I've seen the selective imports isn't as common, it has the effect of defining what should be public in a different place from where things are actually defined. Using __all__ keeps everything about a definition in one file.

What MONAI does is a bit of a mix at the moment, what I had implemented before was an export decorator to fill the role of __all__ but I think now we're better off without it, though still using the auto-import load_submodules routine. What we don't want is the statement from monai import * to import every definition into the current namespace, there needs to be some hierarchy somewhere still.

[rijobro]

Couldn't figure out how to set flake8 up so that import * was allowed inside of __init__.py provided __all__ was defined in the import module. In the end I simply removed all import *.

[wyli]

thanks, looks good to me.

optionally could add a sentence in CONTRIBUTING.md about adding new function/class?

(also doesn't setting per-file-ignores work? https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-per-file-ignores

MONAI/setup.cfg

Line 67 in db7bf89

per-file-ignores = __init__.py: F401

)

[rijobro]

optionally could add a sentence in CONTRIBUTING.md about adding new function/class?

Will do.

(also doesn't setting per-file-ignores work? https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-per-file-ignores

We could ignore all __init__.py files, but that's not really what we want (we want to flag up an error if import * is used with no __all__).

[wyli]

/integration-test

[wyli]

thanks it looks good to me.