Support Python 3 in addition to Python 2

[pavoljuhas]

• make sure pickles are created with protocol 2 otherwise PY3-saved projects would not load in PY2

[dragonyanglong]

Hi @pavoljuhas , I finished the test on 2.7, 3.5, 3.6 and 3.7, and the republic release PDFgui 1.1.2. I tested on both Linux and Mac OS. They gave the same error. I listed below.

Part I: install the support-python-3 branch pdfgui in Py27, 35, 36, and 37. Run in development mode. Create a project and save the ddp file. Try use other py version pdfgui (and the public release PDFgui 1.1.2) to open the four .ddp files. For example, use Py37 pdfgui to create a new project and save into ddp file, then use Py27, 35, 36 to open it.

If use Py27 PDFgui or the public release PDFgui 1.1.2 to open .ddp files generated by 35, 36, and 37, it shows an error message box:

Invalid or corrupted project /home/ly0/Documents/billinge/dev/test_pdfgui_py3/project_py37.ddp.

I screenshot the .ddp file generated by Py37 PDFgui here as an example.

Other cases are fine. Like Py 37 PDFgui can successfully open the .ddp files generated by Py27, 35, 36 PDFgui.

Part II: install the support-python-3 branch pdfgui in Py27, 35, 36, and 37. Open Emil’s old tutorial ddp files in these four version pdfgui, and go through the tutorial(https://gitlab.thebillingegroup.com/tutorials/1809_xpdpdfworkshop/tree/master/PDFgui-practical/PDFgui-tutorial).

Py37, Py36, Py35

1. Copy function. Right click one fit, phase or data, click “copy”. It gives:

Traceback (most recent call last): same in mac
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/gui/errorwrapper.py", line 59, in _f
    return func(*args, **kwargs)
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/gui/mainframe.py", line 1707, in onCopy
    self.treeCtrlMain.CopyBranch(selections[0])
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/gui/fittree.py", line 560, in CopyBranch
    cdatastring = "pdfgui_cliboard=" + cdatastring
TypeError: can only concatenate str (not "bytes") to str

2. Cannot load a Ni structure from file. Create new fit, add phase, open a Ni cif file (Ni.cif in the Emil’s tutorial.). (https://gitlab.thebillingegroup.com/tutorials/1809_xpdpdfworkshop/blob/master/PDFgui-practical/PDFgui-tutorial/04-Ni-Si-mixture/Ni.cif) It gives:

Traceback (most recent call last):
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/gui/errorwrapper.py", line 59, in _f
    return func(*args, **kwargs)
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/gui/addphasepanel.py", line 163, in onOpen
    insertafter=self.entryphase, filename=self.fullpath)
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/gui/fittree.py", line 411, in AddPhase
    self.control.loadStructure(pdata, filename, label, pos)
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/control/pdfguicontrol.py", line 221, in loadStructure
    struct.initial.read(filename)
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/control/fitstructure.py", line 114, in read
    p = PDFStructure.read(self, filename, format)
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/control/pdfstructure.py", line 53, in read
    p = PDFFitStructure.read(self, filename, format)
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/site-packages/diffpy.structure-3.0.1-py3.7.egg/diffpy/structure/pdffitstructure.py", line 58, in read
    p = Structure.read(self, filename, format)
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/site-packages/diffpy.structure-3.0.1-py3.7.egg/diffpy/structure/structure.py", line 236, in read
    new_structure = p.parseFile(filename)
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/site-packages/diffpy.structure-3.0.1-py3.7.egg/diffpy/structure/parsers/p_auto.py", line 89, in parseFile
    return self._wrapParseMethod("parseFile", filename)
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/site-packages/diffpy.structure-3.0.1-py3.7.egg/diffpy/structure/parsers/p_auto.py", line 110, in _wrapParseMethod
    stru = pmethod(*args, **kwargs)
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/site-packages/diffpy.structure-3.0.1-py3.7.egg/diffpy/structure/parsers/structureparser.py", line 80, in parseFile
    s = fp.read()
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/codecs.py", line 322, in decode
    (result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xa9 in position 17: invalid start byte

Other cif files like Si.cif in the tutorial seems like working fine.

3. Sometimes, After create a fit tree in the panel. Right click the fit, click “insert a phase” or “insert dataset”, it shows the following. But I cannot reproduce this error everytime. It seems like usually happen when I open the Py27 version PDFgui first, then open the Py37 version. Maybe this error was just the “.pyc” files when I run in development mode left in the repo:

Traceback (most recent call last):
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/gui/errorwrapper.py", line 59, in _f
    return func(*args, **kwargs)
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/gui/adddatapanel.py", line 95, in readConfiguration
    remember = self.cP.getboolean("DATASET", "remember")
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/configparser.py", line 828, in getboolean
    raw=raw, vars=vars, fallback=fallback, **kwargs)
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/configparser.py", line 808, in _get_conv
    **kwargs)
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/configparser.py", line 802, in _get
    return conv(self.get(section, option, **kwargs))
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/configparser.py", line 1160, in _convert_to_boolean
    raise ValueError('Not a boolean: %s' % value)
ValueError: Not a boolean: ['["[\'False\']"]']

4. Select several cells in phase panel. If Press “shift” (or “command” in mac) and select a few cells, it works fine. But if press “control” on keyboard, click a few cells, for example one row of U33 ADPs. it shows:

Traceback (most recent call last):
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/gui/errorwrapper.py", line 59, in _f
    return func(*args, **kwargs)
  File "/home/ly0/Documents/billinge/dev/test_pdfgui_py3/diffpy.pdfgui/src/diffpy/pdfgui/gui/phaseconfigurepanel.py", line 453, in onEditorShown
    self._selectedCells = gridutils.getSelectedCells(self.gridAtoms)
  File "/home/ly0/anaconda3/envs/pdfgui_py37_test/lib/python3.7/site-packages/diffpy.utils-3.0.0-py3.7.egg/diffpy/utils/wx/gridutils.py", line 78, in getSelectedCells
    rcset.update(grid.GetSelectedCells())
TypeError: unhashable type: 'GridCellCoords'

5. Doping series Macro is not working (Though I guess this problem was pre-exist for a long time before)

6. The phase configure panel, shows “grey” on phase configuration boxes. (No constraints were applied yet)
   

Py27

1. Copy function bug also exists for Py27. Though the error message is slightly different.

Traceback (most recent call last):
  File "diffpy/pdfgui/gui/errorwrapper.py", line 59, in _f
    return func(*args, **kwargs)
  File "diffpy/pdfgui/gui/mainframe.py", line 1707, in onCopy
    self.treeCtrlMain.CopyBranch(selections[0])
  File "diffpy/pdfgui/gui/fittree.py", line 561, in CopyBranch
    textdata = wx.TextDataObject(cdatastring)
UnicodeDecodeError: 'utf8' codec can't decode byte 0x80 in position 16: invalid start byte

2. Cannot add phase/data into a fit tree. After create a fit tree in the panel. Right click the fit, click “insert a phase” or “insert dataset”, it shows the following:

Traceback (most recent call last):
  File "diffpy/pdfgui/gui/errorwrapper.py", line 59, in _f
    return func(*args, **kwargs)
  File "diffpy/pdfgui/gui/addphasepanel.py", line 104, in readConfiguration
    remember = self.cP.getboolean("PHASE", "remember")
  File "/home/ly0/anaconda3/envs/pdfgui_py27_test/lib/python2.7/ConfigParser.py", line 369, in getboolean
    if v.lower() not in self._boolean_states:
AttributeError: 'list' object has no attribute 'lower'

Interesting, after I got this error in Py27 pdfgui. I go back to Py35,36,37 version, it also has this same error. I am not sure whether it is because some mixing between configuration of different pdfgui version installed under development mode. This bug doesn’t exist when I test on mac OS.

3. After close the error dialog window shown in issue 2 above. If continue choose “load a structure from file”, select “Ni.cif” in tutorial, it gives:

Unable to read file 'Ni.cif'
Unknown or invalid structure format.
Errors per each tested structure format:
cif: Star Format error: /home/ly0/Documents/billinge/dev/test_pdfgui_py3/1809_xpdpdfworkshop/PDFgui-practical/PDFgui-tutorial/02-Ni-Neutron/Ni.cif: bad encoding (must be utf8 or ascii)
discus: 249: unit cell not defined
pdb: 1: invalid record name ''data_64989-ICSD''
pdffit: 249: file is not in PDFfit format
rawxyz: 1: invalid RAWXYZ format, expected 3 or 4 columns
xcfg: 1: first line must contain 'Number of particles ='
xyz: 1: invalid XYZ format, missing number of atoms.

[dragonyanglong]

I attach here the ddp files created and saved by Py37 in Linux and Mac during my testing.

Archive.zip

[dragonyanglong]

Hi @pavoljuhas , hope all is well. I chatted with @sbillinge about the incompatibility problem the pickel between py2 and py3. He suggests that if it is a big problem, it is okay for py3 version not to load old pdfgui ddp files. How do you think about merging this PR? Or maybe merge it into dev branch, and release it as beta version for more testing on this? Thanks.

[pavoljuhas]

Hi @dragonyanglong and @sbillinge, the tests above seem to say that project saved from this version installed for 3.7 does not load with 2.7. I suppose a way to go would be to support only Python 3 once this is merged. There are however other errors that should be fixed, e.g., loading of tutorial CIF files. The errors which mention configparser may be due to incompatibility or corruption of the .pdfgui.cfg configuration file. Try to remove that file from your HOME if it helps.
Sorry, I have otherwise no time to follow up here.

[sbillinge]

Hi @dragonyanglong and @sbillinge, the tests above seem to say that project saved from this version installed for 3.7 does not load with 2.7. I suppose a way to go would be to support only Python 3 once this is merged. There are however other errors that should be fixed, e.g., loading of tutorial CIF files. The errors which mention configparser may be due to incompatibility or corruption of the .pdfgui.cfg configuration file. Try to remove that file from your HOME if it helps.
Sorry, I have otherwise no time to follow up here.

Thanks @pavoljuhas! Let's drop python 2 support (it is EOL'd anyway), then just fix the other errors if possible.

[dragonyanglong]

Hi @pavoljuhas , hope everything goes well. I am working on fixing the issues for this PR. How would you suggest to merge this PR?

You merge this PR to pdfgui repo first, then I create PR from my own forked repo to pdfgui repo independently, or I directly push my commits to this PR (i.e. to your forked repo's branch). Or other suggestions?

Thanks so much!

[pavoljuhas]

Hi @dragonyanglong, there may be users who install from master so we should perhaps keep this separate so things don't break for them. I suggest we merge this to the next branch and then you can create new PR-s towards the next branch. Once the code in next is usable, you can just merge next to master and continue development in a standard way.

Are you OK with such plan? If so, I can take care of merging this PR to next.

[sbillinge]

all good with me.

…

On Wed, Nov 18, 2020 at 1:40 AM Pavol Juhas ***@***.***> wrote: Hi @dragonyanglong <https://github.com/dragonyanglong>, there may be users who install from master so we should perhaps keep this separate so things don't break for them. I suggest we merge this to the next branch and then you can create new PR-s towards the next branch. Once the code in next is usable, you can just merge next to master and continue development in a standard way. Are you OK with such plan? If so, I can take care of merging this PR to next. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#42 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABAOWUNIQOJS2CRKJ45LGLDSQNT4RANCNFSM4I25B3BA> .

-- Simon Billinge Professor, Columbia University Physicist, Brookhaven National Laboratory

[dragonyanglong]

Hi @pavoljuhas , sounds good to me. Thanks for helping with the merging. Let's go with it then.

BTW, I was stuck at a bug about the gui for quite long time, but still couldn't find any relevant info online.
In this Py3 version, the phase panel will show "Grey" even if no constrains are applied yet.
So when we launch pdfgui in the development mode it would be like this:

However, if I use wxGlade software to open the corresponding phaseconfigurepanel.wxg, it looks just like normal:

Do you have any insights on this weird behavior?

[pavoljuhas]

@dragonyanglong - merge to the next done. Please do follow-up PRs towards the next branch.

the phase panel will show "Grey" even if no constrains are applied yet.

There is a code that greys-out and locks input fields with active constraints.
Try to disable it and see if PDFgui still starts with fields greyed out.

[dragonyanglong]

Thanks so much, Pavol. I can now create PR to the next branch for further edits successfully.

Regarding the grey background color in parameter boxes, thanks for the hint, I found this change-background-color function at https://github.com/diffpy/diffpy.pdfgui/blob/master/src/diffpy/pdfgui/gui/phaseconfigurepanel.py#L252-L269. If we disable it, no problems anymore. So I suspect it is due to when using newer wxpython version, which changes from
txtbg = self.textCtrlA.GetDefaultAttributes().colBg (py2) to txtbg = self.textCtrlA.DefaultStyle.BackgroundColour (py3).

I was wondering how did you decide to change these wxpython commands when upgrading from py2 to py3, just googling the function one by one? Or any automatic process?
I currently fixed this bug by forcing textCtrl.SetBackgroundColour(txtbg) to be textCtrl.SetBackgroundColour(wx.WHITE), but this is not that smart as your original code, that's why I would like to know how you upgrade wxpython codes from py2 to py3?

[sbillinge]

@dragonyanglong should this PR be closed and you will make a new one into next? Please advise.

[pavoljuhas]

I was wondering how did you decide to change these wxpython commands when upgrading from py2 to py3, just googling the function one by one? Or any automatic process?

Hi Long, this is not quite Python 2 vs 3 issue, instead wxPython had some incompatible changes in version 4 (Phoenix). PDFgui was developed in wxPython <= 3.0. I mostly followed this guide to adjust wxPython calls - https://wxpython.org/Phoenix/docs/html/MigrationGuide.html. You may want to skim over #34 if there are some reusable changes.

I currently fixed this bug by forcing textCtrl.SetBackgroundColour(txtbg) to be textCtrl.SetBackgroundColour(wx.WHITE), but this is not that smart as your original code, that's why I would like to know how you upgrade wxpython codes from py2 to py3?

txtbg should change depending on whether the GUI field is constrained or not.
I guess you need to track down why it has incorrect value for unconstrained fields.

[sbillinge]

5/21 - found that these commits are all merged into branch next .

Leave this for now, and close later, after we merge next into master when everything is working on next