Skip to content
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

Update lesson extracting-keywords from Python 2 to Python 3. #1470

Conversation

frederik-elwert
Copy link
Contributor

This updates one lesson as discussed in #1460.

@acrymble
Copy link

These changes are all very minor code or link changes so I'm happy for them to be merged. I'm not an expert on Python 3 though so I wouldn't be able to say with confidence that these changes were sufficient to make this fully Python 3 compliant. @mdlincoln do you have a technical suggestion here? Should we (probably me) retry the code entirely before approving, as a workflow for these Python updates? If it becomes a substantial re-write we can trigger a re-review or consider retirement? I suspect it's mostly just minor syntax stuff.

@mdlincoln
Copy link
Contributor

@acrymble yes, it's a good idea for you to fully re-run all the code to confirm that it functions.

@acrymble
Copy link

acrymble commented Oct 3, 2019

Thank you so much for this @frederik-elwert . I've re-tried the code and it works fine. I do have one query though. Do you have to explicitly run python3? Does this line need updating?

You can now run the program you’ve written with the following command:

python extractKeywords.py

TO:

You can now run the program you’ve written with the following command:

python3 extractKeywords.py

?

@frederik-elwert
Copy link
Contributor Author

That's a good point. I'm afraid this depends on the system and setup. On Ubuntu using the default Python, you're right, you'd have to run python3. With anaconda, it installs it's Python 3 as python. I'm currently on the road, but I'll try to think of something. Maybe a note in the general Python into lesson that we can point to.

@acrymble
Copy link

acrymble commented Oct 3, 2019

I guess it doesn't matter in this case? I don't think you made any changes that wouldn't work for Python 2, and int ime, this will resolve itself as fewer people have Python2 installed.

@frederik-elwert
Copy link
Contributor Author

You are right, doesn't matter in this case and does not need to block the merge. Good to keep in mind for further lessons, though.

@acrymble acrymble merged commit 7d2416c into programminghistorian:gh-pages Oct 4, 2019
@frederik-elwert frederik-elwert deleted the issue-1460-extracting-keywords branch October 15, 2019 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants