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

🚸 Confusing name choice: lamin_connect() and connect() #138

Open
falexwolf opened this issue Jan 21, 2025 · 2 comments
Open

🚸 Confusing name choice: lamin_connect() and connect() #138

falexwolf opened this issue Jan 21, 2025 · 2 comments

Comments

@falexwolf
Copy link
Member

Apologies that this slipped for me before.

How about renaming what's called laminr::lamin_connect() to laminr::connect_default() or laminr::set_default_db() or laminr::set_default_instance() (in a backward compatible way)?

Otherwise the direct sequence of laminr::lamin_connect() and laminr::connect() is somewhat confusing, because they're both performing "lamin connect".

Image

Or alternatively, eliminating laminr::lamin_connect() from the API and instead have a flag set_as_default = False in laminr::connect()?

Among the 5 designs here, I believe I'd like laminr::connect_default() the most.

@lazappi
Copy link
Collaborator

lazappi commented Jan 28, 2025

I named the functions that way to match the CLI commands so lamin_connect() is equivalent to lamin connect and lamin_login() is equivalent to lamin login. I figured it was good to keep the names the same because the functions are just wrappers around the CLI calls. When can rename it if you like but then we lose that link.

The connect() function is quite different because it is making API calls rather than interacting with Python/CLI.

The overlap in names is unfortunate but I think it will be resolved when we move away from the API as I don't think we will need the connect() function then.

@falexwolf
Copy link
Member Author

Got it, makes sense. Let's keep things this way for now.

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

No branches or pull requests

2 participants