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

Move all 2nd layer import to 1st layer to form pub APIs #6722

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Jan 22, 2025

As discussed in #6636, if we are going to take more aggressive change to make aiida module import more "numpy-like" which means only the first level import regard as the public APIs.

We were talking frequently about backward-compatibility but if the public APIs are not clear, it is a bit meaningless to say backward compatibility and bind our feet to make changes confidently.

It is a long list and we need to take a look at what should not be exposed and what should add to the list.
I'll draft an AEP for this as well.

For the plugin APIs for community to implement the plugins. If we treat it as dependency injection pattern, then we should clear mark all the abstract methods or clear using python protocol to say which methods are expected and any changes on those contract should be regarded as breaking changes.

pinning @giovannipizzi since we discuss this yesterday and @sphuber @agoscinski redirect from #6636, what do you think?

@danielhollas
Copy link
Collaborator

Citing from #6636

I agree, especially after the discussion during the coding week with @agoscinski and @khsrali, I also read https://benhoyt.com/writings/python-api-design/. One of the take home message there is "Design your library to be used as import lib ... lib.Thing() rather than from lib import LibThing ... LibThing().". For aiida case is using from aiida import orm and then orm.Int is not so good since the "orm" is very domain specific and new users will confused with the terminologies.

Just a quick note: also in relation to #6722 -- AiiDA's API surface is huge. I don't know if having everything under top level is tenable, especially in terms of IDE's support. It also goes against the changes where some of the things on Node objects were namespaced under the node.base.
(obviously I was not part of the discussions so don't have the full context)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants