search

mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.66k stars 1.31k forks source link

Consider using absolute imports #12710

Open cbrnr opened 1 month ago

cbrnr commented 1 month ago

I'd like to suggest to use absolute instead of relative imports. Absolute imports are more readable and in general provide better error messages, so PEP8 (and I 😄) recommend(s) using them (https://peps.python.org/pep-0008/#imports). Most of the work can probably be done by Ruff (https://docs.astral.sh/ruff/rules/relative-imports/).

hoechenberger commented 1 month ago

FWIW MNE-BIDS has been using absolute imports for many years, maybe @sappelhoff would care to share the reasoning behind and experience with that?

I do agree that the relative imports in MNE are most often very much unreadable: two, three, sometimes four leading dots .... is just nothing my brain is capable of processing efficiently

2nd FWIW: For all my own projects I'm using absolute imports in combination with a src layout. This has the positive effect that it's now basically impossible to accidentally work with a messed-up development installation (or to forget installation altogether), as imports from the non-installed packages will now immediately fail, which is not the case when using relative imports and a flat package hierarchy.

cbrnr commented 1 month ago

Although this is a separate (but related) issue, I'm also +💯 on switching to a src layout. I've also used both things (absolute imports and src layout) in MNELAB and SleepECG without any problems (not surprisingly, because these are the recommended approaches nowadays).

agramfort commented 1 month ago

Although this is a separate issue, I'm also +💯 on switching to a src layout. I've also used both things (absolute imports and src layout) in MNELAB and SleepECG without any problems (not surprisingly, because these are the recommended approaches nowadays).

numpy, scipy, scikit-learn, joblib don't use src layout...

no objection with absolute imports

Message ID: @.***>

hoechenberger commented 1 month ago

Sorry about conflating this issue / request here, let's maybe not consider / discuss src layout for now and focus on absolute imports only :)

sappelhoff commented 1 month ago

FWIW MNE-BIDS has been using absolute imports for many years, maybe @sappelhoff would care to share the reasoning behind and experience with that?

I have always tried to recommend / enforce this (when I had the decision power), for the same reasons that Clemens mentions. :-) I would be very happy if we adopted this in MNE-Python, too.

hoechenberger commented 1 month ago

I have always tried to recommend / enforce this (when I had the decision power),

Yes, it was actually you who made me research this topic a little after you rejected my proposal to switch MNE-BIDS to relative imports a couple of years back or so :) this is when I figured that absolute imports are actually officially recommended.

larsoner commented 1 month ago

Okay with me to migrate to absolute imports. It'll probably create a lot of PR conflicts so we should make sure we don't have any big pending PRs open when we do it (or at least be aware of what they are so we can consciously decide to accept the rebase/merge pain)

drammock commented 1 month ago

Okay with me to migrate to absolute imports.

no strong feeling. When doing so we should update our import/nesting tests to detect and fail if anyone puts in a relative import.