py3k support #81
py3k support #81
Conversation
| @@ -39,7 +39,7 @@ def _get_synapse_by_name(self, synapse_name): | |||
| """ | |||
| all_synapse = self.brain.brain_yaml | |||
| for el in all_synapse: | |||
| print el | |||
| print(el) | |||
There was a problem hiding this comment.
We use python 2 only. Parentheses are not necessary.
There was a problem hiding this comment.
And also this print should not be here, it a debug print that should has been pushed into the logger.
There was a problem hiding this comment.
I'm just allowing the code to not emit syntax errors here
There was a problem hiding this comment.
In python 2 it's not a syntax error. My IDE doesn't alert me about any error anyway. Did you configure you IDE to use python 2.7 interpreter?
There was a problem hiding this comment.
The python3 interpreter does not allow this old syntax, this one is compatible with both python2 and python3.
|
Thank you but we don't support python 3 and it is not in our scope so far. |
|
this makes the code compatible with py3k (and still works on python2 of course) |
|
Yes, but only for this part it doesn't make sense. If we have to do it, we must do it for the whole code. |
|
Which part did I miss ? |
|
I haven't check yet if there is another part that need to be converted. |
|
I thinks it's all ok for the syntax fixes. All packages are available for python3 except pygmail and jinja. |
|
An issue just fall about a problem with a lib on python 3... |
|
Turns out it was jinja2 imported and not jinja, I updated the modules in setup.py accordingly (updated as well ipadress & ansible). |
|
I will test this on Python 3 soon . We need to check that everything is Le 20 nov. 2016 10:41 PM, "Julien Schueller" notifications@github.com a
|
|
It was much easier for me to test in this branch, this change is very useful. |
| 'ansible==2.1.1.0', | ||
| 'python2-pythondialog==3.4.0', | ||
| 'jinja==1.2', | ||
| 'ansible==2.2.0.0', |
There was a problem hiding this comment.
Not tested yet, and there is a lot of broken modules in this version. I follow the Ansible repo.
There was a problem hiding this comment.
I can revert this one and propose it once all is merged and you have a ci running.
|
Currently, the entry point is broken, I have to fix this and then test everything is still working before merging. Also, there is 2 other PR about neurons improvement which there will be merged before. This branch will to merge those changes. Last thing, the pygmail lib is not python 3 ready. Did you try it? |
|
I applied this patch against master: charlierguo/gmail#48 and at least the import is ok. |
|
Humm, ok. And so what is your proposition about that matter? To keep a version of this lib in our project? To propose a doc of how to patch the lib? |
|
Yes, I guess the project is dead, but the first thing to do is email the author, maybe he'll give you the access to his repo. I'm clearly against including the lib into kalliope ; we may have to fork (there's a lot of work to review PRs, fix issues, and upload to pypi!). |
|
Ok, we'll see. As I said, Supporting Python 3 is totally out of our scope right now. This interpreter is only native in Fedora 23. And we already have a lot of thing to do on Kalliope. |
|
I understand, at least it should be a link to the patch. |
|
Hi, Yes as @Sispheor just mentionned, Python 3 is not in the scope now. It will imply much reviews and lib checking (like Gmail). Plus, I think we need to make an other branch to manage this improvement in the future. We can not interact with the feature on the "review_install" branch. |
|
It doesn't hurt to have this preliminary work merged though ; this doesn't imply for kalliope to advertise for python3 support. |
|
It will not be merged soon. See #75 |
…tead of 'from core'
No description provided.