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

absolufy-imports - No relative imports - PEP8 #7204

Merged
merged 12 commits into from
Dec 7, 2022

Conversation

Illviljan
Copy link
Contributor

@Illviljan Illviljan commented Oct 24, 2022

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.

@Illviljan Illviljan changed the title absolufy imports absolufy-imports - No relative imports - PEP8 Oct 24, 2022
@max-sixty
Copy link
Collaborator

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.

@Illviljan Illviljan added topic-testing CI Continuous Integration tools plan to merge Final call for comments Automation Github bots, testing workflows, release automation and removed topic-dask topic-plotting topic-performance topic-cftime topic-zarr Related to zarr storage library topic-arrays related to flexible array support topic-html-repr topic-rolling needs discussion topic-combine combine/concat/merge run-benchmark Run the ASV benchmark workflow topic-typing io labels Dec 7, 2022
@Illviljan Illviljan enabled auto-merge (squash) December 7, 2022 21:08
@Illviljan Illviljan merged commit 6e77f5e into pydata:main Dec 7, 2022
@dcherian
Copy link
Contributor

dcherian commented Dec 7, 2022

Thanks for the contribution and the patience! @Illviljan

dcherian added a commit to dcherian/xarray that referenced this pull request Dec 8, 2022
* main:
  absolufy-imports - No relative imports - PEP8 (pydata#7204)
  [skip-ci] whats-new for dev (pydata#7351)
  Whats-new: 2022.12.0 (pydata#7345)
  Fix assign_coords resetting all dimension coords to default index (pydata#7347)
@dcherian dcherian added run-benchmark Run the ASV benchmark workflow labels Dec 8, 2022
@@ -1,8 +1,7 @@
import pandas as pd

import xarray as xr

from . import parameterized, randn, requires_dask
from asv_bench.benchmarks import parameterized, randn, requires_dask
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for using absolufy-imports - just for reference, in pandas we don't use it on asv_bench

https://github.com/pandas-dev/pandas/blob/4eb4729fb47bd160a39ec49f6af89dabfc63d3ac/.pre-commit-config.yaml#L8-L12

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

Illviljan added a commit to Illviljan/xarray that referenced this pull request Dec 8, 2022
dcherian pushed a commit that referenced this pull request Dec 9, 2022
* Revert "absolufy-imports - No relative imports - PEP8 (#7204)"

This reverts commit 6e77f5e.

* absolufy-imports - No relative imports - PEP8

* absolufy-imports - No relative imports - PEP8

* absolufy-imports - No relative imports - PEP8

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update whats-new.rst

* Revert "[pre-commit.ci] auto fixes from pre-commit.com hooks"

This reverts commit 9f50f5d.

* Update .pre-commit-config.yaml

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@Illviljan Illviljan deleted the absolufy_imports branch December 10, 2022 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Automation Github bots, testing workflows, release automation CI Continuous Integration tools plan to merge Final call for comments run-benchmark Run the ASV benchmark workflow topic-testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants