DjangoGirls / tutorial

This is a tutorial we are using for Django Girls workshops
http://tutorial.djangogirls.org/
Other
1.53k stars 1.86k forks source link

Python 3 Upgrade #1683

Closed nikhiljohn10 closed 3 years ago

nikhiljohn10 commented 3 years ago

Draft for Python 3 upgrade

If this draft is acceptable, i'll create a PR using these commits.

@das-g @ekohl Please review. Thanks

nikhiljohn10 commented 3 years ago

I got some questions:

1.

2.

3.

das-g commented 3 years ago

1.

  • [ ] Do I have to create screenshots for the new upgrade within this PR regarding Python? -or-
  • [ ] Should I create a new PR will new screenshots of every screenshots found inside tutorial?

Whichever you prefer. Third option:

That's the idea behind preparing the updates that depend on each other on a separate branch: We don't have to cover everything in a single PR, and more than one person can work on different aspects.

das-g commented 3 years ago

2.

  • [ ] Manually hard code version of python in tutorial? -or-
  • [ ] Automate Python versions using book.json and variable replacement?

Automating it with the variable might be a good idea, especially to keep the lesser-active translation more in sync. Though, it can result in these translations getting internally inconsistent, if stuff that depends on the version but hasn't been automated doesn't get re-translated.

Would 4863b610f306bb91b2495a6fe2f4872ebaf06ad3 already cover all occurrences of the Python version number(s)?

das-g commented 3 years ago

3.

  • [ ] Does python intro need any changes according to Python 3.8 version? -or-
  • [ ] Should I not touch the python Intro tutorial at all?

As far as I know, Python 3.8 and 3.9 are fully backwards-compatible to the Python version(s) we currently use, i.e., everything that will work in Python 3.6 should work in 3.8 and 3.9, too. So there is no need to update Python intro. (We should probably test it with the new Python version(s), but I don't expect to find any differences in behavior except maybe for slightly different output formats here and there.)

While I very much like the newly introduced features of the newer Python versions, I don't think any of them are relevant enough for newcomers that they'd have to be covered in the Intro. If we ever come to a different conclusion, we can still make that change then, in a new PR.

nikhiljohn10 commented 3 years ago

Would 4863b61 already cover all occurrences of the Python version number(s)?

@das-g Yes, it covers 100% of version texts inside the tutorial and converts to the GitBook variable.

Does that mean to commit in this draft PR is ok approval?

If yes, I don't need a new PR. I can change the draft status of this PR for review and merge. There is nothing else to update regarding python versions.

nikhiljohn10 commented 3 years ago

On file django_installation/instructions.md, Line 176:

(myvenv) ~$ python -m pip install --upgrade pip

Can simply use pip install -U pip instead? I have tested and both commands result in upgrading pip inside myvenv directory.

das-g commented 3 years ago

On file django_installation/instructions.md, Line 176:

(myvenv) ~$ python -m pip install --upgrade pip

Can simply use pip install -U pip instead? I have tested and both commands result in upgrading pip inside myvenv directory.

I'm not sure. The python3 -m part was added in #1274 (and changed to the python -m variant without version number in #1450). @anapaulagomes, @ekohl can you remember what the problem was with plain pip install --upgrade pip instead of python -m pip install --upgrade pip?

IIRC the python -m pip install --upgrade pip variant proved to work on more different system (various Linux distributions, probably) than plain pip install --upgrade pip. Why that might have been, I'm not quite sure, as within a Python 3 virtualenv, the pip command should always be available, whether pip installed system-wide or not. (Some Linux distributions package pip separately from Python, thus it's possible that system-wide, Python 3 is installed, but the pip for it isn't. But in a virtualenv, that shouldn't be an issue.)

das-g commented 3 years ago

On file django_installation/instructions.md, Line 176:

(myvenv) ~$ python -m pip install --upgrade pip

Can simply use pip install -U pip instead? I have tested and both commands result in upgrading pip inside myvenv directory.

As for -U vs. --upgrade: Although it's more to type, I'd stick with the long option, as it's more obvious what it might do than with the short one, for which a new user would probably have to consult the documentation.

nikhiljohn10 commented 3 years ago

As for -U vs. --upgrade: Although it's more to type, I'd stick with the long option, as it's more obvious what it might do than with the short one, for which a new user would probably have to consult the documentation.

I don't find that as a valid statement since there are other short forms of options used inside tutorial. For eg, -r, --requirement in pip install, -u, --set-upstream in git push etc. When I started learning python, i have also google all these to understand what they really do.

We can add a 'NOTE' under pip install -U p about its -U option is --upgrade shortform.

How about that?

nikhiljohn10 commented 3 years ago

On file django_installation/instructions.md, Line 176:


(myvenv) ~$ python -m pip install --upgrade pip

Can simply use pip install -U pip instead? I have tested and both commands result in upgrading pip inside myvenv directory.

I'm not sure. The python3 -m part was added in #1274 (and changed to the python -m variant without version number in #1450). @anapaulagomes, @ekohl can you remember what the problem was with plain pip install --upgrade pip instead of python -m pip install --upgrade pip?

IIRC the python -m pip install --upgrade pip variant proved to work on more different system (various Linux distributions, probably) than plain pip install --upgrade pip. Why that might have been, I'm not quite sure, as within a Python 3 virtualenv, the pip command should always be available, whether pip installed system-wide or not. (Some Linux distributions package pip separately from Python, thus it's possible that system-wide, Python 3 is installed, but the pip for it isn't. But in a virtualenv, that shouldn't be an issue.)

I am sure this is only for Windows OS. So lets try the approach we used in this link. https://tutorial.djangogirls.org/en/django_models/#creating-an-application

Here both options are given in separate command line sections.

For eg:

(Linux and MacOS Users)

(myvenv) ~ $ pip install -U pip

(Windows Users)

(myvenv) C:\> python -m pip install -U pip

@das-g let me know your thoughts on above method.

anapaulagomes commented 3 years ago

Sorry, I don't remember. 😐

das-g commented 3 years ago

Would 4863b61 already cover all occurrences of the Python version number(s)?

@das-g Yes, it covers 100% of version texts inside the tutorial and converts to the GitBook variable.

:+1:

Does that mean to commit in this draft PR is ok approval?

I'm not sure I understand your question. Can you rephrase it? (If English isn't your native language, maybe write it in your native language.)

If yes, I don't need a new PR. I can change the draft status of this PR for review and merge. There is nothing else to update regarding python versions.

Yes, converting a draft PR to a normal, merge-able PR is completely OK. You don't have to file a new PR for the same thing.

das-g commented 3 years ago

We can add a 'NOTE' under pip install -U p about its -U option is --upgrade shortform.

Yeah, with such an explanation (be it in a note admonition or just in the prose) it should be fine. Feel free to open a separate PR for that. (Separate, as it's independent from the python version change.)

das-g commented 3 years ago

About python -m pip ... vs. just pip ...:

I don't think it was a Windows specific issue. On Windows, too, a virtualenv created by python -m venv should include pip. And when the virtualenv is activated, the pip from the virutalenv should be available as simply pip.

After some research, I guess the real reason is, that Python 3's venv module didn't always install pip:

(Though I'm not quite sure how the pip module (which is used by -m pip) could have been available then, as it isn't a standard library module, and wasn't one in Python 3.3, either.)

Since Django 3.1 requires Python ≥ 3.6 we can probably drop the workaround for all operating systems. (Though, we should test on all of them, first.)

nikhiljohn10 commented 3 years ago

About python -m pip ... vs. just pip ...:

I don't think it was a Windows specific issue. On Windows, too, a virtualenv created by python -m venv should include pip. And when the virtualenv is activated, the pip from the virutalenv should be available as simply pip.

After some research, I guess the real reason is, that Python 3's venv module didn't always install pip:

(Though I'm not quite sure how the pip module (which is used by -m pip) could have been available then, as it isn't a standard library module, and wasn't one in Python 3.3, either.)

Since Django 3.1 requires Python ≥ 3.6 we can probably drop the workaround for all operating systems. (Though, we should test on all of them, first.)

Hope this helps

Packaging status

das-g commented 3 years ago

Sounds good to me.

On November 14, 2020 3:57:23 PM GMT+01:00, Nikhil John notifications@github.com wrote:

@nikhiljohn10 commented on this pull request.

-If you have a different version of Python installed, at least 3.4.0 (e.g. 3.6.0), then you don't have to upgrade. If you don't have Python installed, or if you want a different version, first check what Linux distribution you are using with the following command: +If you have a different version of Python installed, at least 3.4.0 (e.g. {{ book.py_release }}), then you don't have to upgrade. If you don't have Python installed, or if you want a different version, first check what Linux distribution you are using with the following command:

That's a great idea. How does the {{ book.py_min_version }} sounds?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/DjangoGirls/tutorial/pull/1683#discussion_r523427963

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

nikhiljohn10 commented 3 years ago

I have generated a stagging site under my personal domain. This contains all the above commits.

https://django.nikz.in/en/

ekohl commented 3 years ago

@das-g shall we merge it or is there anything else to do?

das-g commented 3 years ago

@das-g shall we merge it or is there anything else to do?

I'm not aware of anything more that would have to be done for changing the Python version(s).

According to its release notes, the currently used Django version (2.2) does support both Python versions the tutorial will mention with this PR: Python 3.6 & Python 3.8. Thus it should be fine to merge this right into master instead of (or maybe better, additionally to) merging it into django-3.1.

I didn't yet have time to test the complete tutorial with any of the new Python versions, and probably won't have time for that in the near future. So if anyone did test it or will test it, please comment here. Nonetheless, I'm confident that this shouldn't introduce any major problems.

Shall I go ahead an merge it, @ekohl ? (Or, if you agree, feel free to merge it yourself.)

ekohl commented 3 years ago

I didn't yet have time to test the complete tutorial with any of the new Python versions, and probably won't have time for that in the near future. So if anyone did test it or will test it, please comment here. Nonetheless, I'm confident that this shouldn't introduce any major problems.

I'm in a similar position so I was also a bit hesitant to merge. However, like you I'm also confident it won't introduce major problems. Let's merge it.

Thanks @nikhiljohn10!