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

#208 Updating readme to add badge #209

Merged
merged 6 commits into from
Dec 23, 2024
Merged

Conversation

Conor0Callaghan
Copy link
Contributor

Add badge to README as per #208 part 1

Add badge to README as per pittcsc#208 part 1
Copy link
Member

@rohit-ganguly rohit-ganguly left a comment

Choose a reason for hiding this comment

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

Thanks for adding the shield, that's awesome. I think it would be useful to add a short section around installation via pip/pipenv too towards the start of the doc!

Copy link
Contributor

@tianyizheng02 tianyizheng02 left a comment

Choose a reason for hiding this comment

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

For convenience, is it possible to have the badge be a clickable link to PittAPI's PyPI page?

Updating Readme with first attempt at additional install commands as per comments in pittcsc#208
@Conor0Callaghan
Copy link
Contributor Author

Thanks for adding the shield, that's awesome. I think it would be useful to add a short section around installation via pip/pipenv too towards the start of the doc!

hopefully that's a good start!

Copy link
Contributor

@tianyizheng02 tianyizheng02 left a comment

Choose a reason for hiding this comment

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

Personally I don't think we need to include instructions for setting up a virtual environment in the Installation section. I think it's better to leave that choice of using a virtual environment (and how to set one up) up to the user, rather than recommending it with pipenv specifically. I feel that it's enough to explain how to install the package itself through pip.

README.md Outdated Show resolved Hide resolved
@tianyizheng02 tianyizheng02 linked an issue Dec 20, 2024 that may be closed by this pull request
@timparenti
Copy link
Member

Personally I don't think we need to include instructions for setting up a virtual environment in the Installation section. I think it's better to leave that choice of using a virtual environment (and how to set one up) up to the user

Since many of the intended audience for this API are beginners and learners, I disagree. I think it's important to explicitly let folks at that level know of the option so that they can make an informed decision on whether they want to "pollute" their default dependency tree, or at least be aware that that could happen in the likely case they want to keep something else separate later.

Now, could this text maybe be a bit clearer that a virtual environment is not a requirement, but merely a best practice that's worth considering? Absolutely. Should it instead just point people to good write-ups on how to set up the most common types instead of providing step-by-step instructions for one option ourselves? Perhaps. But, to my mind, that adds a dose of unnecessary friction, and for something this simple, I don't think the proposed instructions are harmful as written.

Beginners have to be introduced to these concepts somehow so they can experiment and, as is your goal, ultimately make their own choices. I think this project is a fantastic low-stakes place for folks to encounter and weigh these considerations, if they haven't before.

README.md Outdated Show resolved Hide resolved
@azharichenko azharichenko merged commit cbe748b into pittcsc:dev Dec 23, 2024
4 checks passed
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.

README.md does not have link to PyPI package
5 participants