Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor: use absolute imports #252

Merged
merged 3 commits into from
Aug 10, 2021
Merged

Conversation

alexander-held
Copy link
Member

@alexander-held alexander-held commented Aug 10, 2021

Motivated by scikit-hep/pyhf#1539, this switches from relative to absolute imports within cabinetry. Reasons are outlined in scikit-hep/pyhf#1539 (comment), reproduced below:

The imports section of PEP 8 says

Absolute imports are recommended, as they are usually more readable and tend to be better behaved (or at least give better error messages) if the import system is incorrectly configured

and

However, explicit relative imports are an acceptable alternative to absolute imports, especially when dealing with complex package layouts where using absolute imports would be unnecessarily verbose

PEP 8 used to discourage relative imports more strongly.

The Google style guide section says

Do not use relative names in imports. Even if the module is in the same package, use the full package name. This helps prevent unintentionally importing a package twice.

The relative imports used previously were originally motivated by pyhf, so it seems appropriate to switch to absolute imports together with pyhf.

* switch from relative to absolute imports

@codecov
Copy link

codecov bot commented Aug 10, 2021

Codecov Report

Merging #252 (f58ce92) into master (1043103) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #252   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         1720      1720           
  Branches       261       261           
=========================================
  Hits          1720      1720           
Impacted Files Coverage Δ
src/cabinetry/__init__.py 100.00% <100.00%> (ø)
src/cabinetry/cli/__init__.py 100.00% <100.00%> (ø)
src/cabinetry/fit.py 100.00% <100.00%> (ø)
src/cabinetry/route.py 100.00% <100.00%> (ø)
src/cabinetry/template_builder.py 100.00% <100.00%> (ø)
src/cabinetry/template_postprocessor.py 100.00% <100.00%> (ø)
src/cabinetry/visualize/__init__.py 100.00% <100.00%> (ø)
src/cabinetry/visualize/plot_model.py 100.00% <100.00%> (ø)
src/cabinetry/visualize/plot_result.py 100.00% <100.00%> (ø)
src/cabinetry/workspace.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1043103...f58ce92. Read the comment docs.

@alexander-held alexander-held merged commit 12b4af9 into master Aug 10, 2021
@alexander-held alexander-held deleted the refactor/absolute-imports branch August 10, 2021 13:10
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant