General python support #145

VikingScientist · 2018-06-28T08:24:51Z

In awaiting the conclusion from #141, I tried to show how a general version-agnostic approach could look. In practice, you only need to add the line from __future__ import print_function to all python files and they will in 99% of the times work for both versions. At least when considering small algorithms such as this project here and not huge complex interconnected libraries that have lots of dependencies.

leios · 2018-06-28T21:26:11Z

Thanks for the PR. Quick question, since we have decided to get rid of Python 2 in #141 , is this code immediately mergable, or is it just an example of how we can make the code version agnostic?

EDIT: I have just been told I was incorrect about what this PR does, It corrects python 2 code to work with python3 syntax, right? So this PR might go away if we get rid of Python 2.

jiegillet · 2018-06-29T01:31:39Z

I think the consensus is that we don't want to have a compromise on versions. Getting rid of py2 and py3 and use py is great, but I don't think we want to include from __future__ import print_function at all. People who want to use python 3 code using python 2 should add that themselves.

jiegillet · 2018-06-29T07:03:56Z

chapters/euclidean_algorithm/euclidean.md

@@ -89,7 +89,7 @@ Program.cs
 [import, lang="javascript"](code/javascript/euclidean_example.js)
 {% sample lang="py2" %}


You missed a py2 here

jiegillet · 2018-06-29T07:06:31Z

chapters/tree_traversal/tree_traversal.md

@@ -41,7 +39,7 @@ Because of this, the most straightforward way to traverse the tree might be recu
 {% sample lang="js" %}
 [import:15-23, lang:"javascript"](code/javascript/Tree_example.js)
 {% sample lang="py2" %}


jiegillet · 2018-06-29T07:07:15Z

chapters/tree_traversal/tree_traversal.md

@@ -41,7 +39,7 @@ Because of this, the most straightforward way to traverse the tree might be recu
 {% sample lang="js" %}
 [import:15-23, lang:"javascript"](code/javascript/Tree_example.js)
 {% sample lang="py2" %}
-[import:8-16, lang:"python"](code/python2/Tree_example.py)
+[import:7-15, lang:"python"](code/python/Tree_example.py)
 {% sample lang="py3" %}


py3 here (although there is no code)

leios · 2018-06-29T07:39:37Z

As a quick note the py label should be defined in the book.json file in the root directory. this will make sure that all py labels will have the correct tag at the top of the screen ("Python" instead of "Python 2" or "Python 3"). I can do this and submit a PR to your branch @VikingScientist , or you can change

                                {
                                        "lang": "py2",
                                        "name": "Python 2"
                                },
                                {
                                        "lang": "py3",
                                        "name": "Python 3"
                                },

to

                                {
                                        "lang": "py2",
                                        "name": "Python 2"
                                },
                                {
                                        "lang": "py3",
                                        "name": "Python 3"
                                },
                                {
                                        "lang": "py",
                                        "name": "Python"
                                },

and then just check to make sure the language is correct and syntax highlighting is right by running gitbook serve. I figure we should leave py2 and py3 in there for now and get rid of them once everything's updated.

Note: Indentation was from file.

jiegillet · 2018-06-29T07:41:26Z

@leios is there a place we could mention explicitly our stance on python version?

EDIT: Wait, can I edit other people's comments? That's amazing.

leios · 2018-06-29T07:44:34Z

@jiegillet I think I we already did in #141. Maybe we should make language-specific style guides and mention it in there?

EDIT: How do you feel when I edit YOUR comments, mister?

jiegillet · 2018-06-29T07:47:57Z

No I mean in the book. The topic is bound to come up. It should be stated somewhere, maybe?

leios · 2018-06-29T07:49:54Z

Yeah, that's what I meant too. We should add some style guidelines to the How to Contribute section and for python, we just say that we are sticking to python 3 for this.

I am not sure where else to put it, but @julianschacher was mentioning he wanted to create a How to Contribute guide... So that would be a good place too.

Maybe we should move this to a separate issue?

jiegillet · 2018-06-29T07:51:43Z

No #141 is the right place to discuss that. Here is not though, so let's get back to code reviewing, sorry @VikingScientist

VikingScientist · 2018-06-29T08:01:31Z

Hehe. This is fine. I initiated the discussion, and if it is discussed here or in #141 really doesn't bother me. I agree with @leios though: we should put the "python 3 only"-stance somewhere (i.e. in the How to contribute section) so that it is available for new contributers. The issue tracker will fade; in fact auto-close after this PR is merged, so it's not a good long-term documentation.

There's a few upcoming commits more where I make the requested changes to book.json as well as fixing the last python-code in the FFT. After this, all version-specific code is gone, so I'll remove py2 and py3 from book.json.

Edit: just have to get this gitbook-things installed and running so I can verify that it all works locally.

jiegillet · 2018-06-29T08:04:13Z

Coolz. Ping me when you feel it's ready for a merge.

leios · 2018-06-29T08:04:35Z

Oh yeah, if this PR removes all version specific stuff from the whole book, it should be fine to remove py2 and py3. I just thought there were more python implementations hiding somewhere.

VikingScientist · 2018-06-29T08:10:45Z

I did grep for any mention of py2 or py3, but can't find any trace of it anymore.

I like fixup commits such as 2356130 as it makes the reviewers life easier, but I want these to be squashed with it's referencing commit before being merged into trunk. So if I get a thumbs up on this so far then I'll run git rebase -i HEAD~6 --autosquash and then it's ready to be merged.

Butt4cak3 · 2018-06-29T09:10:42Z

We always squash and merge PRs (through the GH interface) anyway. There's no need to manually squash. But thanks for the consideration!

VikingScientist · 2018-06-29T09:21:44Z

Rebased to resolve some merge conflicts. Ready to be looked at @jiegillet

leios · 2018-06-29T11:17:40Z

Great!, so once this is merged, we can have a look at the python 3 implementations #146 #144 #138 , all of which should have modifications in their markdown files.

I am mentioning this now so we remember on those later.

Butt4cak3

I think that's it! I'm pretty sure I checked everything, so let's merge this and get going with the other Python PRs.

This was referenced Jun 28, 2018

Adding Python 3 euclidean example #138

Closed

Added python Bogo Sort #142

Merged

Butt4cak3 added Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) Implementation Edit This provides an edit to an algorithm implementation. (Code and maybe md files are edited.) labels Jun 28, 2018

Butt4cak3 mentioned this pull request Jun 28, 2018

Add bubble_sort.py to bubble_sort.md #154

Closed

jiegillet reviewed Jun 29, 2018

View reviewed changes

VikingScientist added 8 commits June 29, 2018 10:14

Moved Tree traversal to a combined py2/py3 implementation

b61244c

Python implementaiton of bubble sort is now shown, also tab fixes

03ce35a

Euclidean algorithm moved for general python (version 2/3)

11bc987

Fixes algorithm-archivists#141: removes support for python 2

043b752

fixup! Fixes algorithm-archivists#141: removes support for python 2

c91c7b5

Moved python verlet code to only py3 (henceforth known as py)

eddada4

Moved python 2 FFT code to py

f15244b

fixup! Fixes algorithm-archivists#141: removes support for python 2

777c512

VikingScientist force-pushed the MorePython branch from 2356130 to 777c512 Compare June 29, 2018 09:20

Butt4cak3 approved these changes Jun 29, 2018

View reviewed changes

Butt4cak3 merged commit 908e7b6 into algorithm-archivists:master Jun 29, 2018

VikingScientist deleted the MorePython branch June 29, 2018 14:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

General python support #145

General python support #145

VikingScientist commented Jun 28, 2018

leios commented Jun 28, 2018 •

edited

Loading

jiegillet commented Jun 29, 2018

jiegillet Jun 29, 2018

jiegillet Jun 29, 2018

jiegillet Jun 29, 2018

leios commented Jun 29, 2018 •

edited

Loading

jiegillet commented Jun 29, 2018 •

edited by leios

Loading

leios commented Jun 29, 2018 •

edited by jiegillet

Loading

jiegillet commented Jun 29, 2018

leios commented Jun 29, 2018

jiegillet commented Jun 29, 2018 •

edited

Loading

VikingScientist commented Jun 29, 2018 •

edited

Loading

jiegillet commented Jun 29, 2018

leios commented Jun 29, 2018

VikingScientist commented Jun 29, 2018

Butt4cak3 commented Jun 29, 2018

VikingScientist commented Jun 29, 2018

leios commented Jun 29, 2018

Butt4cak3 left a comment

		@@ -89,7 +89,7 @@ Program.cs
		[import, lang="javascript"](code/javascript/euclidean_example.js)
		{% sample lang="py2" %}

General python support #145

General python support #145

Conversation

VikingScientist commented Jun 28, 2018

leios commented Jun 28, 2018 • edited Loading

jiegillet commented Jun 29, 2018

jiegillet Jun 29, 2018

Choose a reason for hiding this comment

jiegillet Jun 29, 2018

Choose a reason for hiding this comment

jiegillet Jun 29, 2018

Choose a reason for hiding this comment

leios commented Jun 29, 2018 • edited Loading

jiegillet commented Jun 29, 2018 • edited by leios Loading

leios commented Jun 29, 2018 • edited by jiegillet Loading

jiegillet commented Jun 29, 2018

leios commented Jun 29, 2018

jiegillet commented Jun 29, 2018 • edited Loading

VikingScientist commented Jun 29, 2018 • edited Loading

jiegillet commented Jun 29, 2018

leios commented Jun 29, 2018

VikingScientist commented Jun 29, 2018

Butt4cak3 commented Jun 29, 2018

VikingScientist commented Jun 29, 2018

leios commented Jun 29, 2018

Butt4cak3 left a comment

Choose a reason for hiding this comment

leios commented Jun 28, 2018 •

edited

Loading

leios commented Jun 29, 2018 •

edited

Loading

jiegillet commented Jun 29, 2018 •

edited by leios

Loading

leios commented Jun 29, 2018 •

edited by jiegillet

Loading

jiegillet commented Jun 29, 2018 •

edited

Loading

VikingScientist commented Jun 29, 2018 •

edited

Loading