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

Review public APIs (make sure they are "Pythonic") #101

Closed
1 task
toddbaert opened this issue Apr 19, 2023 · 7 comments
Closed
1 task

Review public APIs (make sure they are "Pythonic") #101

toddbaert opened this issue Apr 19, 2023 · 7 comments
Labels
1.0-release Required for a 1.0 release sdk

Comments

@toddbaert
Copy link
Member

We know some of the internals of the SDK are a bit "Java-y"; these can be refactored later. However, before a 1.0 we need to make sure the public APIs are idiomatic to ensure we don't regret or design choices post-1.0.

Definition of done:

  • Review SDK, issues created for public API changes if necessary.
@federicobond
Copy link
Member

The biggest issue is see here is that the module structure is too deeply nested. I have no hard data on this, but in my experience one should be able to import 80-90% of the code that is needed for most use cases from open_feature.<xyz> and only very specialized functionality from open_feature.<xyz>.<abc>

Some examples:

Original import Proposed import
from open_feature.hooks.hook import Hook from open_feature.hooks import Hook
from open_feature.provider.no_op_provider import NoOpProvider from open_feature.provider import NoOpProvider

If needed, additional internal structure can be handled via underscore-prefixed modules to signal that you should not import these directly.

The other Java-ism I am seeing is the open_feature.open_feature_<x> module naming, closely following the main class name in each case. A more Pythonic approach would be to rename them to open_feature.client and open_feature.api.

@toddbaert
Copy link
Member Author

@federicobond the above sounds right to me.

Because this is an issue of style, I'd love to hear some concensus from others. cc @hlipsig @matthewelwell @jamescarr @tcarrio @open-feature/sdk-python-approvers @open-feature/sdk-python-maintainers

@hlipsig
Copy link
Collaborator

hlipsig commented Jul 10, 2023

Agreed.

@beeme1mr
Copy link
Member

Sounds like a nice change to me 👍.

@matthewelwell
Copy link
Member

Yep, agree.

@gruebel
Copy link
Member

gruebel commented May 16, 2024

@federicobond should we close this?

@federicobond
Copy link
Member

Yes, I find no more directly actionable issues here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0-release Required for a 1.0 release sdk
Projects
None yet
Development

No branches or pull requests

6 participants