Skip to content

Review lectures #255

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
Merged
merged 11 commits into from
Jul 3, 2023
Merged

Review lectures #255

merged 11 commits into from
Jul 3, 2023

Conversation

@DiaPorntipa
Copy link
Contributor

Hello @jstac, this PR contains all minor changes of lectures 3 and 4 edited by @HengchengZhang , @orectique and me.
@DiaPorntipa DiaPorntipa requested a review from jstac June 29, 2023 05:02
@netlify
Copy link

netlify bot commented Jun 29, 2023 •
edited
Loading

✅ Deploy Preview for taupe-gaufre-c4e660 ready!

Name Link
🔨 Latest commit 255cad1
🔍 Latest deploy log https://app.netlify.com/sites/taupe-gaufre-c4e660/deploys/64a24a86defc560008a293de
😎 Deploy Preview https://deploy-preview-255--taupe-gaufre-c4e660.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.
@HengchengZhang
Copy link
Member

Thanks @DiaPorntipa for creating the PR, but there's still some changes I need to commit before merging.
@github-actions
Copy link

github-actions bot commented Jun 29, 2023 •
edited
Loading

🚀 Deployed on https://64a24ed7bad6fb40384128a8--taupe-gaufre-c4e660.netlify.app
jstac
jstac reviewed Jun 29, 2023
View reviewed changes
jstac
jstac reviewed Jun 29, 2023
View reviewed changes
jstac
jstac reviewed Jun 29, 2023
View reviewed changes
@jstac
Copy link
Contributor

jstac commented Jun 29, 2023

Thanks @DiaPorntipa ! Very helpful.

Please see the small requests above.
@HengchengZhang
Copy link
Member

Hi @jstac, I've applied the review changes and make other modifications.
@jstac
Copy link
Contributor

jstac commented Jul 2, 2023

Thanks @HengchengZhang , nice work.

I think this code block should be hidden:

# transfer the survey weights from absolute into relative values ...

And in the code block below it, we should stick to PEP8 --- in particular, lines should be 80 characters max, so those long function calls need to be spread across two lines.
@HengchengZhang
Copy link
Member

Hi @jstac, I see that's because there's error in codes and now they are fixed.

Similar errors are in LA lecture as well and I will fix it next reading group.
@DiaPorntipa
Copy link
Contributor Author

Thanks @HengchengZhang for the update.
@jstac
Copy link
Contributor

jstac commented Jul 3, 2023

Nice work @DiaPorntipa and @HengchengZhang . Merging.
@jstac jstac merged commit 6030b83 into main Jul 3, 2023
@jstac jstac deleted the review_lectures branch July 3, 2023 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready

4 participants

@DiaPorntipa @HengchengZhang @jstac @orectique