-
-
Notifications
You must be signed in to change notification settings - Fork 42
Sheffield | 25-SDC-Nov | Hassan Osman | Sprint 4 | Implement cowsay in Python #267
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
base: main
Are you sure you want to change the base?
Sheffield | 25-SDC-Nov | Hassan Osman | Sprint 4 | Implement cowsay in Python #267
Conversation
…esult of the arguments with spaces in between
…an end user can select from
|
Can you ignore the .venv file? It is difficult to review this as there are too many files in the PR. thanks |
Hi! I've just ignored it. Please let me know if there's anything else needs changing. Appreciate your help! :) |
LonMcGregor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start, i've left some comments where you could improve further
| import sys | ||
| import argparse | ||
|
|
||
| listed_animals = [listed_animal for listed_animal in dir(cowsay) if callable(getattr(cowsay, listed_animal)) and not listed_animal.startswith("__") and listed_animal not in ["draw", "func", "get_output_string", "CowsayError"]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might work, but looking over every single item in dir(cowsay) and manually excluding the ones you dont want might not be very maintainable in the long term. Is there an easier way to access this information?
| args = parser.parse_args() | ||
|
|
||
| message = " ".join(args.message) | ||
| animal = args.animal or "cow" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are setting a default argument here which is good, but perhaps there may be a neater way of doing this. Think about where the rest of the arguments are handled.
Self checklist
In this PR I Implemented a command-line tool using the cowsay library with dynamic animal options, message input, validation, and proper --help output.