Skip to content

minor fixes #222

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

Merged
merged 3 commits into from
Jun 19, 2024
Merged

minor fixes #222

merged 3 commits into from
Jun 19, 2024

Conversation

d-kleine
Copy link
Contributor

  • improved GH issues template (labels must be array-like)
  • ch07: fixed typo "evalyate" - "evaluate"

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@rasbt
Copy link
Owner

rasbt commented Jun 18, 2024

Thanks for the PR! I am wondering about the \n characters that are added everywhere. Based on ReviewNB, they show up as added whitespaces. What's the purpose of it?

@d-kleine
Copy link
Contributor Author

\n are line breaks, you can ignore them if you want to keep it as compact as possible

@rasbt
Copy link
Owner

rasbt commented Jun 18, 2024

\n are line breaks

Yeah, but why adding them to the headers? I feel like this is a bit unnecessary and seems to add unnecessary white spaces here

Screenshot 2024-06-18 at 3 23 15 PM

@d-kleine d-kleine marked this pull request as draft June 18, 2024 21:26
@d-kleine d-kleine force-pushed the main branch 2 times, most recently from f07f787 to 8b1ee68 Compare June 18, 2024 21:30
@d-kleine
Copy link
Contributor Author

Yeah, doesn't make sense. I think one of my extensions is broken.
I have reset the PR to changing only what's relevant.

@d-kleine d-kleine marked this pull request as ready for review June 18, 2024 21:43
@d-kleine
Copy link
Contributor Author

d-kleine commented Jun 18, 2024

BTW I also checked the wget command for the Issues template. Might be problematic for Windows users as they need to install wget separately. Worked for me with

curl --ssl-no-revoke -O https://raw.githubusercontent.com/rasbt/LLMs-from-scratch/main/setup/02_installing-python-libraries/python_environment_check.py

(--ssl-no-revoke due to SSL/TLS certificate validation errors on Windows)

@d-kleine
Copy link
Contributor Author

Also the python_environment_check.py only works in subfolders, but not when saved in the root dir

@rasbt
Copy link
Owner

rasbt commented Jun 19, 2024

Thanks, these are good points. Just fixed these things for Windows (and made the subfolder thing work).

@rasbt rasbt merged commit fba2059 into rasbt:main Jun 19, 2024
5 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.

2 participants