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

Jump to bottom

[docs] Footgun: install function should not fail when given a string. #1091

Closed
3 tasks done
JosephCatrambone opened this issue Sep 25, 2024 · 0 comments
Closed
3 tasks done

[docs] Footgun: install function should not fail when given a string. #1091

JosephCatrambone opened this issue Sep 25, 2024 · 0 comments
Labels
documentation Improvements or additions to documentation

Comments

@JosephCatrambone
Copy link
Contributor

JosephCatrambone commented Sep 25, 2024 •
edited
Loading

Description
from guardrails.cli.hub.install import install is a footgun on two levels.

Current documentation
Right now, if one does from guardrails.cli.hub.install import install and install("hub://guardrails/nsfw_text"), the function will show, "Invalid URI! The package URI must start with 'hub://'". In reality, install is expecting a LIST of hub URIs, so the "hub://" is decomposed into ['h', 'u', 'b', ...].

Additionally, one should be using from guardrails.hub import install.

Suggested changes
First, we should detect if a string is passed and warn about using directly, second, we should rename the install CLI function to install_cli or something else. The actual command line instruction can be the same, guardrails hub install is good, but we should rename the def so people don't import it by mistake.

Checklist

  • I have checked that this issue hasn't already been reported
  • I have checked the latest version of the documentation to ensure this issue still exists
  • For simple typos or fixes, I have considered submitting a pull request instead
@JosephCatrambone JosephCatrambone added the documentation Improvements or additions to documentation label Sep 25, 2024
JosephCatrambone added a commit that referenced this issue Sep 25, 2024
@JosephCatrambone
Fix for #1091. Make the install CLI function named install_cli (but k…
ab4550a 
…eep the invocation as guardrails hub install) so people don't import it by mistake. Add warning for string passage.
@JosephCatrambone JosephCatrambone mentioned this issue Sep 25, 2024
Merged
JosephCatrambone added a commit that referenced this issue Sep 25, 2024
@JosephCatrambone
[Fix] #1091. Prevent confusion between guardrails.cli.hub.install and…
9b7cfc3 
… guardrails.hub install. (#1093)

* Fix for #1091. Make the install CLI function named install_cli (but keep the invocation as guardrails hub install) so people don't import it by mistake. Add warning for string passage.

* Fix typo in docs in install.py. Correct a reference.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@JosephCatrambone