Skip to content
This repository has been archived by the owner on Mar 17, 2021. It is now read-only.

Support for python 3 (+python 2) #703

Merged
merged 121 commits into from
Oct 19, 2018
Merged

Support for python 3 (+python 2) #703

merged 121 commits into from
Oct 19, 2018

Conversation

piertoni
Copy link

After applying futurize stage1 I need some help on the following:

  • Update Travis CI script for Python3
  • on lib/taurus/qt/qtgui/display/qled.py i changed the following line in order to pass the test:
#def __init__(self, parent=None, ledsize=LedSize.SMALL, ledcolor=LedColor.GREEN):
def __init__(self, parent=None, ledsize=24, ledcolor=LedColor.GREEN):

It's an enumeration problem as the enumeration is not starting from 0.
For what I have seen can be something related to Taurus Enumerations that must start from 0.
Surely will be straightforward for some of you that knows the codebase well.

@cpascual
Copy link
Member

cpascual commented Feb 23, 2018 •
edited
Loading

Hi @piertoni , thanks for the PR.
Right now the @taurus-org/integrators are in a rush for finishing the Jan18 release of Taurus and Sardana. Please give us a couple of weeks so that we can review this PR with the calm it deserves.

For the moment I'll mark it with "Help Wanted" in the hope of attracting attention from somebody else who may be interested in reviewing this in the meanwhile.

Expression 'x' in Enumeration failes on python3 with KeyError.
de3d3da 
To solve this problem, explicit __contains__ method is added.
@cpascual cpascual mentioned this pull request Jun 28, 2018
Closed
@cpascual
Copy link
Member

cpascual commented Jun 28, 2018 •
edited
Loading

Hi @piertoni another user (@asenchenko) is also doing some effort on supporting python3 (see #765 ).
I wonder if you are still interested in pushing for this feature, and if so, if you would agree to centralise the contributions to this in your branch. This would be more agile since all the work could go on autonomously (no need for official @taurus-org/integrators attention until the moment of actually doing a PR
to the official develop branch with the final implementation.

Alternatively, if you all feel more comfortable this way, we can also create a py3 branch in the official repo and all python3-related PRs could go there (but then the PRs will be delayed by the official integrators' time availability).

Carlos Pascual added 2 commits June 28, 2018 14:03
Merge branch 'futurize1' of https://github.com/piertoni/taurus into p…
058203d 
…iertoni-futurize1

Fixed conflicts in:
	doc/buildmock.py
	lib/taurus/core/tango/tangodatabase.py
Add python3 tests to Travis
7210b56 
Use the debian-stretch-py3 docker image to test taurus with the
python3 stack
@cpascual
Copy link
Member

Hi, I updated this branch with the latest changes from develop (so that it does not have merge conflicts) and I also configured travis to do tests for python3

Carlos Pascual added 3 commits June 28, 2018 17:19
Fix typo in image name in travis
75decce
Merge branch 'enum_fixes' of https://github.com/asenchenko/taurus int…
8cc9a3f 
…o piertoni-futurize1
Revert workaround on use of Enumeration
d532274 
QLedOld's init kwarg ledsize was changed to an int in a previous commit
as a workaround for python3 support, but the last commit should fix it
without need of workaround, so we revert the workaround.
@cpascual
Copy link
Member

I merged the changes from #765 here. and reverted the workaround on qled.py mentioned in the first post of the commit

@cpascual
Copy link
Member

The testsuite is erroring now for the python3 case: see: https://travis-ci.org/taurus-org/taurus/jobs/397882052

Carlos Pascual and others added 5 commits June 28, 2018 19:05
@cpascual
Copy link
Member

Note: I did 2 commits ( 99d83c2 , 1b58868 ) replacing imports of Queue by six.moves.queue. But now I think it is best to revert them and let futurize treat them (which will result in using the future module instead of six

So, just to be clear: IMHO, we should officially state that the python3 support should be implemented by following the instructions in:
http://python-future.org/automatic_conversion.html#futurize-quick-start-guide

@piertoni
Copy link
Author

Ok for using my branch and for the help of @asenchenko or any other.

@cpascual
Copy link
Member

cpascual commented Jul 2, 2018 •
edited
Loading

Ok for using my branch and for the help of @asenchenko or any other.

Great, so let's do that.

Therefore, @asenchenko, please propose your PRs directly to this branch (piertoni:futurize1)

And let's use this PR (#703) for coordinating

@cpascual cpascual added this to the Jan19 milestone Jul 2, 2018
@cpascual
Copy link
Member

Hi, I finished with the step 4 of the roadmap.

It would be nice now that as much people as possible would test the resulting code both with py2 and with py3 (in steps 5 and 7 we will use the checklist that we use for releases, but still, there are many things that may not appear in that checklist).

Carlos Pascual added 2 commits September 27, 2018 16:19
Group imports
7fcb1e0 
The futurize call left some "messy" imports (e.g. blocks of imports
before and after the `__all__` variable, etc. Clean that a bit. Also
remove a few unused imports.
Fix import error
f6d0a4d 
The previous commit introduced a an error in the tango module imports.
Fix it.
@cpascual
Copy link
Member

cpascual commented Sep 28, 2018 •
edited
Loading

Hi to @piertoni , @asenchenko , @taurus-org/integrators and everybody who is interested, this is a call for help in testing this PR under python2 ( step 5 of the roadmap ).

The goal is to have this merged into develop ASAP to have as much exposure as possible of the new code before the Jan19 release.

  • We at alba will run the check-list and more (including tests with sardana) under debian9.
  • Any volunteers for testing under windows?
  • Also, more people testing under linux (debian9 or other systems) would be appreciated.

Finally, the tests can be done with the git checkout, but one can also use the artifacts from appveyor:

@cpascual cpascual requested a review from a team September 28, 2018 08:47
@cpascual cpascual changed the title [WIP] Python3 porting, after Futurize1 Python3 porting, after Futurize1 Sep 28, 2018
@cpascual cpascual changed the title Python3 porting, after Futurize1 Support for python 3 (+python 2) Sep 28, 2018
@piertoni
Copy link
Author

I will do testing on windows and also on linux (debian 8 for the moment), I will report Issues on my repositor if you agree.

@cpascual
Copy link
Member

cpascual commented Oct 1, 2018

I will do testing on windows and also on linux (debian 8 for the moment),

Thanks @piertoni !

I will report Issues on my repositor if you agree.

I propose to open a separate issue for reporting each set of tests (similar to what we did in the last release). If you want to use your repo for the issues, that's fine for me, but using the official repo is also ok.

@asenchenko
Copy link

Hi, I'm currently busy. I could test on Ubuntu 18.04 LTS and debian 9.

This was referenced Oct 4, 2018
Merged
Closed
@teresanunez teresanunez mentioned this pull request Oct 8, 2018
Closed
79 tasks
cmft and others added 5 commits October 9, 2018 14:33
@cmft
Fix issue with SetFormatter after py3 support refactoring
60080fc 
Add missing import and fix the calls of isinstance with string.
Use with future.utils.string_types instead of str.
Fix regresion on taurusdemo buttons
c16db5b 
The for Label, LCD, and LED buttons functionality was broken by the
futurize steps. The problem is related to absolute imports. Fix them.
Fix regresions on taurusmessagebox/panel
bcaee83 
Taurusmessagebox and Taurusmessagepanel are affected by a bug introduced
by futurize. The change from StrinIO -> io   seems to be problematic
on py2 in our case.  As a workaround, just use the old StringIO module
if available.
Fix regresion on albareport
833ee5a 
Fix import issue introduced by futurize in albareport.py.
Fix regresion on icons catalog
8fd1e42 
Fix import str issue introduced in a recent commit
@cpascual
Copy link
Member

The testing effort has started (and some bugs alreay fixed) Here are the issues with the test reports:

The issues being found and which are exclusive to this PR are reported in @piertoni 's tracker: https://github.com/piertoni/taurus/issues

BTW: @piertoni , have you done any tests already on win?

@jkotan
Copy link

jkotan commented Oct 12, 2018

have you try to tests Encoded tango attributes with taurus. It looks like currently they does not work in python3-tango, i.e.: tango-controls/pytango#216 and tango-controls/pytango#213

@cpascual
Copy link
Member

Hi @jkotan , thanks for the tip. I wasn't aware of those bugs. AFAIK, we haven't tested yet (there is no such test in our automated tests nor on our manual checklist).

We will do ASAP, but if you can try it yourself and report here, It would be of great help.

Note that at this moment we are concentrating ensuring no regressions when using python2 (step 5 of our roadmap). If PyTango itself is not yet handling this on python3, I guess we will be affected ourselves, but at least we should ensure that for now, we are not introducing additional problems when running on python2.

cmft and others added 4 commits October 16, 2018 11:44
@cmft
Fix BoundMethodWeakref __eq__ and __ne__ methods
9cef4a8 
The current implementation of these methods are not compatible
for py2 and py3 and cause troubles with the test.

CallableRef method is also wrong adapted

Reimplement the methods
Implement __ne__ based on == instead of __eq__
3420442 
See rationale of the change in https://stackoverflow.com/a/35781654
@cmft
Replace pyqt.Signal str signature
c6c3c56 
To be compatible between Py2 and Py3 we should use 'QString'
instead of str in the signature of the signals.

Do it.
@cmft
Replace isinstance(obj, str)
a819948 
The uses of isinstance with str causes troubles with Py2 and Py3
code compatibility.
This problem can be avoided using `string_types`
(from future.utils) instead of `str`.
cpascual
cpascual approved these changes Oct 19, 2018
View reviewed changes
Copy link
Member

@cpascual cpascual left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done careful review :

All found issues were fixed. and now I consider this ready to be merged.

Note that windows tests have not been done (@piertoni, any news?... if you cannot do them we may do them ourselves) but we think that it is ok to merge now and if later on we find windows-specific regressions, we just treat them as RC bugs before next release

Regarding the comment from @jkotan about encoded attrs, we haven't done a explicit check but the encoded attributes aree used and checked in the sardana testsuite (which is passing) so, for python 2 we expect no regressions.

@cpascual cpascual merged commit 4637f3a into taurus-org:develop Oct 19, 2018
@piertoni
Copy link
Author

Sorry I am a bit full in this period, I will do tests on Windows as soon as possible

@cpascual cpascual mentioned this pull request Apr 2, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@asenchenko asenchenko asenchenko left review comments

@cpascual cpascual cpascual approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
Jan19
Development

Successfully merging this pull request may close these issues.
5 participants
@piertoni @cpascual @asenchenko @jkotan @cmft