-
-
Notifications
You must be signed in to change notification settings - Fork 430
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
Replace print_date function in episode 16 question #546
Conversation
As suggested in #538, the function print_date was previously defined. This updated question will use print_time instead.
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.
thanks for the PR! I've added a few minor comments to be fixed and then I think this will be ready to merge.
_episodes/16-writing-functions.md
Outdated
> | ||
> def print_date(year, month, day): | ||
> joined = str(year) + '/' + str(month) + '/' + str(day) | ||
> def print_date(hour, minute, second): |
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.
I think this needs to be renamed to print_time as well?
_episodes/16-writing-functions.md
Outdated
@@ -203,26 +203,26 @@ result of call is: None | |||
> 1. What's wrong in this example? | |||
> | |||
> ~~~ | |||
> result = print_date(1871,3,19) | |||
> result = print_time(11,37,59) |
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.
Please add spaces between the arguments here (and everywhere else that is missing them) for PEP8 compliance (and consistency with the rest of the lesson code snippets).
_episodes/16-writing-functions.md
Outdated
> print(joined) | ||
> ~~~ | ||
> {: .language-python} | ||
> | ||
> 2. After fixing the problem above, explain why running this example code: | ||
> | ||
> ~~~ | ||
> result = print_date(1871, 3, 19) | ||
> result = print_time(11,37,59) |
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.
need spaces here as well 😅
As noted in #538, the function
print_date
was previously used in the episode, leading to a student to potentially fail to experience the expected error if they type the code into their current python session.This updated question will use
print_time
instead. This is a very simple substitution which I should be as accessible asprint_date
.