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

Make all imports absolute #5318

Merged
merged 1 commit into from Nov 9, 2022
Merged

Conversation

Czaki
Copy link
Collaborator

@Czaki Czaki commented Nov 8, 2022

Description

This is an alternative to #5308 where all imports are absolute

Type of change

  • Refactor

References

How has this been tested?

  • example: the test suite for my feature covers cases x, y, and z
  • example: all tests pass with my change
  • example: I check if my changes works with both PySide and PyQt backends
    as there are small differences between the two Qt bindings.

Final checklist:

  • My PR is the minimum possible work for the desired functionality
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • If I included new strings, I have used trans. to make them localizable.
    For more information see our translations guide.

cc @jni @kephale @goanpeca @psobolewskiPhD

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Nov 8, 2022

As I posted in the other thread, as a serious newbie I find the ... imports really confusing. When reading code I'm never sure where the import is from. Worse yet, I copy-paste a lot to try functions and such to see what they return and do, because I'm not good at reading the code. Then the .. imports don't work and I need to figure out what was intended.
So from my side, as a newbie, relative outsider, and all-around not-very-smart person: 👍

@github-actions github-actions bot added design Design discussion preferences Issues relating to the creation of new preference fields/panels qt Relates to qt tests Something related to our tests labels Nov 8, 2022
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Merging #5318 (90fd73a) into main (8a78b3c) will increase coverage by 0.02%.
The diff coverage is 95.57%.

@@            Coverage Diff             @@
##             main    #5318      +/-   ##
==========================================
+ Coverage   89.07%   89.09%   +0.02%     
==========================================
  Files         581      581              
  Lines       49233    49233              
==========================================
+ Hits        43853    43866      +13     
+ Misses       5380     5367      -13     
Impacted Files Coverage Δ
napari/_app_model/actions/_toggle_action.py 100.00% <ø> (ø)
napari/_app_model/context/_context_keys.py 100.00% <ø> (ø)
napari/_qt/experimental/qt_poll.py 0.00% <0.00%> (ø)
napari/_qt/menus/_util.py 92.45% <ø> (ø)
napari/_qt/widgets/qt_dict_table.py 0.00% <0.00%> (ø)
napari/_vispy/experimental/tile_grid.py 38.46% <ø> (ø)
napari/components/experimental/monitor/_api.py 0.00% <0.00%> (ø)
napari/components/experimental/monitor/_service.py 0.00% <0.00%> (ø)
napari/components/experimental/remote/__init__.py 0.00% <0.00%> (ø)
napari/components/experimental/remote/_commands.py 0.00% <0.00%> (ø)
... and 281 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@goanpeca goanpeca left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @Czaki ! 🚀

Copy link
Member

@jni jni left a comment

Choose a reason for hiding this comment

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

Reading up a bit, it seems that absolute imports are preferred by a large percentage of the Python community, and indeed by our own napari community, so 👍 from me. Thank you @Czaki and also everyone else who has contributed to this discussion!

@brisvag
Copy link
Contributor

brisvag commented Nov 9, 2022

This is gonna be a nightmare on all open PRs ^^'

@Czaki
Copy link
Collaborator Author

Czaki commented Nov 9, 2022

This is gonna be a nightmare on all open PRs ^^'

pre-commit action will solve most of problems

@Czaki
Copy link
Collaborator Author

Czaki commented Nov 9, 2022

Lets the conflicts start.

@Czaki Czaki merged commit ee94c35 into napari:main Nov 9, 2022
@Czaki Czaki deleted the refator_imports_all_absolute branch November 9, 2022 21:32
@Czaki Czaki added the refactor Refactoring code base label Dec 5, 2022
@Czaki Czaki mentioned this pull request Jun 7, 2023
@Czaki Czaki added this to the 0.4.18 milestone Jun 13, 2023
@Czaki Czaki added the maintenance PR with maintance changes, label Jun 13, 2023
Czaki added a commit that referenced this pull request Jun 16, 2023
Czaki added a commit that referenced this pull request Jun 17, 2023
Czaki added a commit that referenced this pull request Jun 18, 2023
Czaki added a commit that referenced this pull request Jun 19, 2023
Czaki added a commit that referenced this pull request Jun 21, 2023
Czaki added a commit that referenced this pull request Jun 21, 2023
Czaki added a commit that referenced this pull request Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design discussion maintenance PR with maintance changes, preferences Issues relating to the creation of new preference fields/panels qt Relates to qt refactor Refactoring code base tests Something related to our tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants