Cape Town| 25-SDC-July | Faith Muzondo | Sprint 4| Implement cowsay#146
Cape Town| 25-SDC-July | Faith Muzondo | Sprint 4| Implement cowsay#146Faithy4444 wants to merge 8 commits intoCodeYourFuture:mainfrom
Conversation
Faithy4444
added
the
Needs Review
illicitonion
changed the title
DaryaShirokova
added
Review in progress
DaryaShirokova
left a comment
DaryaShirokova
left a comment
There was a problem hiding this comment.
Please make sure only files you authored are added to the pr (one main file and one requirements.txt file in this case).
implement-cowsay/main.py
Outdated
| parser.add_argument('message', type=str, help= 'please enter a message') | ||
| arg = parser.parse_args() | ||
|
|
||
| valid_animals = cowsay.char_names |
There was a problem hiding this comment.
There is a way to validate the lsit of animals though argparse directly (check out choices argument).
There was a problem hiding this comment.
I have fixed this and also removed the other files
There was a problem hiding this comment.
Now I think it doesn't validate them at all? Could you please test this out?
Please also validate on the examples in readme:
python3 cow.py Grass, delicious.
python3 cow.py --animal turtle Fish are cool!
python3 cow.py --animal fish Turtles are cool too! // fish is not a valid arg
.vscode/launch.json
Outdated
| @@ -0,0 +1,18 @@ | |||
| { | |||
There was a problem hiding this comment.
There are still some extra files that should not be committed, and README should not be removed, could you please have another look?
There was a problem hiding this comment.
my Apologies. the files were in my main branch thats why I was missing them. I have made changes and also validated using argparse
implement-cowsay/main.py
Outdated
| parser.add_argument('message', type=str, help= 'please enter a message') | ||
| arg = parser.parse_args() | ||
|
|
||
| valid_animals = cowsay.char_names |
There was a problem hiding this comment.
Now I think it doesn't validate them at all? Could you please test this out?
Please also validate on the examples in readme:
python3 cow.py Grass, delicious.
python3 cow.py --animal turtle Fish are cool!
python3 cow.py --animal fish Turtles are cool too! // fish is not a valid arg
implement-cowsay/main.py
Outdated
|
|
||
| parser = argparse.ArgumentParser(description="displays cowsay charactor with given arguments") | ||
| parser.add_argument('animal', type=str, choices=cowsay.char_names, default="cow", help= 'please enter a valid animal') | ||
| parser.add_argument('message', type=str, help= 'please enter a message') |
There was a problem hiding this comment.
Could you please try to run the commands exactly as described in readme? For example, that's what I see (I replaced cow.py with main.py to match your naming):
python3 main.py Grass, delicious.
usage: main.py [-h] {beavis,cheese,cow,daemon,dragon,fox,ghostbusters,kitty,meow,miki,milk,octopus,pig,stegosaurus,stimpy,trex,turkey,turtle,tux} message
main.py: error: argument animal: invalid choice: 'Grass,' (choose from 'beavis', 'cheese', 'cow', 'daemon', 'dragon', 'fox', 'ghostbusters', 'kitty', 'meow', 'miki', 'milk', 'octopus', 'pig', 'stegosaurus', 'stimpy', 'trex', 'turkey', 'turtle', 'tux')
You may want to check nargs for the message arg :) https://docs.python.org/3/library/argparse.html#nargs
You can also check on positional arguments : https://docs.python.org/3/library/argparse.html (animal should be specified via --animal)
Let me know if you have any questions!
DaryaShirokova
added
Complete
|
|
||
| msg = " ".join(arg.message) | ||
| animal_func = getattr(cowsay, arg.animal) | ||
| print(animal_func(msg)) |
There was a problem hiding this comment.
Note: you don't need to use print here as your animal_func already prints the message. Because of the extra print you get None at the end.
2 participants
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.