aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
436 stars 191 forks source link

Make `load_profile` importable from aiida module #6609

Open unkcpz opened 1 week ago

unkcpz commented 1 week ago

fixes #6608

load_profile() is widely used and documented. Exposing it as API under aiida module directly can make a bunch of LSP happy.

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 82.92683% with 7 lines in your changes missing coverage. Please review.

Project coverage is 77.90%. Comparing base (ef60b66) to head (ec71c9f). Report is 133 commits behind head on main.

Files with missing lines Patch % Lines
src/aiida/manage/configuration/profile.py 83.34% 4 Missing :warning:
src/aiida/engine/daemon/client.py 71.43% 2 Missing :warning:
src/aiida/__init__.py 87.50% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6609 +/- ## ========================================== + Coverage 77.51% 77.90% +0.40% ========================================== Files 560 567 +7 Lines 41444 42177 +733 ========================================== + Hits 32120 32854 +734 + Misses 9324 9323 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

danielhollas commented 1 week ago

Exposing it as API under aiida module directly can make a bunch of LSP happy.

Out of curiosity, which LSPs specifically are you talking about?

The load_profile is exposed from aiida package. The __all__ attribute influences only the star imports (from aiida import *). So I don't understand why an LSP should complain.

https://docs.python.org/3/tutorial/modules.html#importing-from-a-package

unkcpz commented 1 week ago

Out of curiosity, which LSPs specifically are you talking about?

The load_profile is exposed from aiida package. The all attribute influences only the star imports (from aiida import *). So I don't understand why an LSP should complain.

I use basedpyright, and I think it is also good to have a list of what APIs we are going to exposed explicitly in all, besides make LSP happy.