absolufy-imports - No relative imports - PEP8#7204
Conversation
for more information, see https://pre-commit.ci
|
I have no particular preference on which, but +0.5 on having a standard. Also PyLance in VSCode has (temporarily, apparently) defaulted to making all imports absolute. So perhaps that will establish a standard. |
|
I'm a bit split on this. As far as I understand it, there were two main reasons for the switch of from .....scheduler import something(which I agree is utter madness), and because they had modules named We're also not in conflict with PEP8: if I read that correctly, it does say they recommend absolute imports, but then it also says that explicit relative imports are fine as well (and not just in the case where a complex structure makes explicit relative imports much more readable). This means that unlike for As such, my vote would be +0.5 for automated consistency, but -0.5 for absolute imports everywhere because for me explicit relative imports are actually a bit easier to read. However, I very much agree with using absolute imports of modules from the main package in tests (I think we have something like |
|
A lot of pep8 is quite gentle with the phrasing I think: Another nice perk with absolute paths is that you can immediately run a |
This reverts commit 66af1c3.
for more information, see https://pre-commit.ci
|
I am merging this on friday next week if no one minds. :) |
|
Since it's a pretty 50/50 change lets discuss this at the next meeting on Dec 7. (I bet changing the tests won't be controversial) |
shoyer
left a comment
There was a problem hiding this comment.
We discussed this in the bi-weekly developers meeting and agreed to go forward, mostly for the sake of consistency with other projects and because it makes it easier to move code around.
|
Thanks for the contribution and the patience! @Illviljan |
| import xarray as xr | ||
|
|
||
| from . import parameterized, randn, requires_dask | ||
| from asv_bench.benchmarks import parameterized, randn, requires_dask |
There was a problem hiding this comment.
This makes our benchmarks fail unfortunately:
STDERR -------->
Error: Traceback (most recent call last):
File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1435, in <module>
main()
File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1428, in main
commands[mode](args)
File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1103, in main_discover
list_benchmarks(benchmark_dir, fp)
File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1088, in list_benchmarks
for benchmark in disc_benchmarks(root):
File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 985, in disc_benchmarks
for module in disc_modules(root_name, ignore_import_errors=ignore_import_errors):
File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 967, in disc_modules
for item in disc_modules(name, ignore_import_errors=ignore_import_errors):
File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 950, in disc_modules
module = import_module(module_name)
File "/home/runner/work/xarray/xarray/asv_bench/.asv/env/3e1d7de4e47af51e3e41695ae1884ae2/lib/python3.8/importlib/__init__.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
File "<frozen importlib._bootstrap>", line 991, in _find_and_load
File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
File "<frozen importlib._bootstrap_external>", line 843, in exec_module
File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
File "/home/runner/work/xarray/xarray/asv_bench/benchmarks/dataarray_missing.py", line 4, in <module>
from asv_bench.benchmarks import parameterized, randn, requires_dask
ModuleNotFoundError: No module named 'asv_bench'
There was a problem hiding this comment.
Hey, thanks for using absolufy-imports - just for reference, in pandas we don't use it on asv_bench
If you add a line to the hook with - files: ^xarray/, and revert the changes to asv_bench, then I think it should work fine
I saw dask has started using absolute imports in dask/dask#8796.
I find it much more readable and there's a nice pre-commit for it as well.
Easiest way to deal with the merge conflicts is probably to just accept your changes and let pre-commit fix them afterwards.