Skip to content

Comments

STYLE: Prefer absolute imports#75

Draft
jhlegarreta wants to merge 1 commit intodemianw:masterfrom
jhlegarreta:PreferAbsoluteImports
Draft

STYLE: Prefer absolute imports#75
jhlegarreta wants to merge 1 commit intodemianw:masterfrom
jhlegarreta:PreferAbsoluteImports

Conversation

@jhlegarreta
Copy link
Contributor

Prefer absoulute import statements: Python PEP 8 recomends using absolute imports:
https://peps.python.org/pep-0008/#imports

"Absolute imports are recommended, as they are usually more readable and tend to be better behaved (or at least give better error messages) (...)."

Prefer absoulute import statements: Python PEP 8 recomends using
absolute imports:
https://peps.python.org/pep-0008/#imports

"Absolute imports are recommended, as they are usually more readable and
tend to be better behaved (or at least give better error messages)
(...)."
@jhlegarreta
Copy link
Contributor Author

Keeping this as a draft until a tag/release is made. xref: #67 (comment)

@demianw
Copy link
Owner

demianw commented Dec 11, 2024

I disagree with this one. Most packages, for instance scikit learn or nilearn, for instance use relative imports.

@demianw
Copy link
Owner

demianw commented Dec 11, 2024

Keeping this as a draft until a tag/release is made. xref: #67 (comment)

Please don't keep them as drafts if you want me to review them, just keep them as draft if they are work in progress. Even if you think we should make a tag/release

@jhlegarreta
Copy link
Contributor Author

Nilearn recommends absolute imports even if they may not be consistent in their code base/may have switched preferences:
https://nilearn.github.io/stable/development.html#contribution-guidelines

scikit-learn recommends relative imports in their code base but absolute imports in unit tests:
https://scikit-learn.org/stable/developers/develop.html#coding-guidelines

I still believe absolute imports are helpful, but do not have other arguments at present.

@jhlegarreta
Copy link
Contributor Author

Please don't keep them as drafts if you want me to review them, just keep them as draft if they are work in progress. Even if you think we should make a tag/release

Was keeping them as drafts to avoid accidentally merging them before tagging. I know tagging can be done from any given commit, but thought it was cleaner this way.

@demianw
Copy link
Owner

demianw commented Dec 11, 2024

Let's keep this one for further discussion and continue moving forward with other PRs. Thanks for such a granular work !

If you look at sklearn's code, all imports are relative e. g. : https://github.com/scikit-learn/scikit-learn/blob/main/sklearn/linear_model/__init__.py

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.

2 participants