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.

Sign up for GitHub

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

Add IDCClient() singleton feature #99

Merged
merged 5 commits into from
Jul 8, 2024
Merged

Add IDCClient() singleton feature #99

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
328e354
feat: make IDCClient directly importable from idc_index
LennyN95 Jul 4, 2024
8f62a7e
feat: pseudo singleton class method
LennyN95 Jul 4, 2024
fccd9d8
Merge branch 'main' into feat_singleton
fedorov Jul 8, 2024
b4ad711
ci: fix lint/ruff errors
fedorov Jul 8, 2024
7f06c57
make IDCClient directly importable
LennyN95 Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions idc_index/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@
from ._version import version as __version__

__all__ = ["__version__"]

from .index import IDCClient
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fedorov I added this import such that we can import IDCClient directly, instead of importing index and then accessing it via index.IDCClient. I see this got removed, is this because we have any rule that prevents adding such imports on the package level or did it just got lost in the process? I prefer this way as it simplifies importing a lot and enhances ux in my opinion but I'm open for counter-arguments ;)

Some of the errors are related to the now missing import, so I wait for your reply before fixing them.

Thx for looking into this!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The linter doesn't like the unused import... I push a workaround, let me know what you think ;)

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, I just accepted the corrections that we proposed by the linter/ruff. Looks good to me!