diffpy / diffpy.pdfgui

graphical user interface for real space structure refinement to PDF
Other
18 stars 27 forks source link

MAINT: update wxGlade panels version #30

Closed dragonyanglong closed 5 years ago

dragonyanglong commented 5 years ago

Update aboutdialog, calculation, datasetconfigure, and output panels to wxGlade v0.8.0b4 and wxpython 2.8. No changes in code function.

codecov[bot] commented 5 years ago

Codecov Report

Merging #30 into maint will not change coverage. The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##            maint      #30   +/-   ##
=======================================
  Coverage   25.22%   25.22%           
=======================================
  Files          88       88           
  Lines       11633    11633           
=======================================
  Hits         2935     2935           
  Misses       8698     8698
Impacted Files Coverage Δ
src/diffpy/pdfgui/gui/outputpanel.py 0% <0%> (ø) :arrow_up:
src/diffpy/pdfgui/gui/aboutdialog.py 0% <0%> (ø) :arrow_up:
src/diffpy/pdfgui/gui/datasetconfigurepanel.py 0% <0%> (ø) :arrow_up:
src/diffpy/pdfgui/gui/calculationpanel.py 0% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8d82902...3623bed. Read the comment docs.

pavoljuhas commented 5 years ago

@dragonyanglong - are you sure wxGlade v0.8.0b4 is the most recent version? There seems to be 0.8.3 which generates a bit more similar code to the one in master.

The code refresh with wxglade erased some hand-made changes to the sources. These were not supposed to be done between #begin wxglade, #end wxglade marks, but unfortunately went in a long time ago and were essential for correct look. I have corrected the year and license text in 218c42c, but the aboutdialog is still missing a hyperlink to PDFgui paper.

Please fix this in a wxglade-consistent way so that "aboutdialog" looks the same as in master. Also please check for any issues with GUI look in all other touched files.

dragonyanglong commented 5 years ago

Hi @pavoljuhas , I was stuck at one error not seen before.

I do python -m diffpy.pdfgui.applications.pdfgui as normal to see the edits. But it always give me:

Traceback (most recent call last):
  File "/home/ly0/anaconda3/envs/pdfgui2/lib/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/home/ly0/anaconda3/envs/pdfgui2/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/home/ly0/Documents/billinge/dev/diffpy.pdfgui/src/diffpy/pdfgui/applications/pdfgui.py", line 129, in <module>
    main()
  File "/home/ly0/Documents/billinge/dev/diffpy.pdfgui/src/diffpy/pdfgui/applications/pdfgui.py", line 121, in main
    guimain.main()
  File "diffpy/pdfgui/gui/Main.py", line 38, in main
    app = PDFGuiApp(0)
  File "/home/ly0/anaconda3/envs/pdfgui2/lib/python2.7/site-packages/wx-3.0-gtk2/wx/_core.py", line 8631, in __init__
    self._BootstrapApp()
  File "/home/ly0/anaconda3/envs/pdfgui2/lib/python2.7/site-packages/wx-3.0-gtk2/wx/_core.py", line 8196, in _BootstrapApp
    return _core_.PyApp__BootstrapApp(*args, **kwargs)
  File "diffpy/pdfgui/gui/Main.py", line 24, in OnInit
    self.frame = MainFrame(None, -1, "")
  File "diffpy/pdfgui/gui/mainframe.py", line 179, in __init__
    self.treeCtrlMain = FitTree(self, -1, style=wx.TR_HAS_BUTTONS|wx.TR_NO_LINES|wx.TR_EDIT_LABELS|wx.TR_MULTIPLE|wx.TR_HIDE_ROOT|wx.TR_MULTIPLE|wx.TR_EXTENDED|wx.TR_DEFAULT_STYLE|wx.SUNKEN_BORDER)
  File "diffpy/pdfgui/gui/fittree.py", line 78, in __init__
    self.fitbmid = il.Add(fitbmp)
  File "/home/ly0/anaconda3/envs/pdfgui2/lib/python2.7/site-packages/wx-3.0-gtk2/wx/_gdi.py", line 6731, in Add
    return _gdi_.ImageList_Add(*args, **kwargs)
wx._core.PyAssertionError: C++ assertion "IsOk()" failed at ./src/gtk/bitmap.cpp(923) in GetWidth(): invalid bitmap

And a window pop out and say "Failed to load image from file "../diffpy.pdfgui/src/icons/calculationitem.png".

Even I git revert back to the master branch newest commits (without my "maint branch" commits), i.e. 8d82902c215c8845c19d32b3abd8692cf774c95d But still the same error.

Do you have any idea about that? Is there something changed when I update the wxGlade version and it cannot be git revert back?

Thank you!

pavoljuhas commented 5 years ago

And a window pop out and say "Failed to load image from file ../diffpy.pdfgui/src/icons/calculationitem.png".

I see the same when I start python.app -m ... in the "src" directory. Just run it in any other directory. It should work then assuming pdfgui was installed in your environment with python setup.py develop -N.

If this is a problem, I can patch icon path resolution so it works in the "src" directory as well.

dragonyanglong commented 5 years ago

Hi @pavoljuhas

Thanks a lot! It works when I change the directory as you said.

I finished the edits on your comments. Please review the PR.

The about dialog (and other dialogs related in the PR) looks the same as master branch now, except the author list (because it is shuffled randomly every time), and the build number (in master branch it is kept 0, and i feel like now it is correct, 76 instead of 0).

If anything incorrect, please let me know. image

dragonyanglong commented 5 years ago

Hi @pavoljuhas , I realized that my next PR to the "new" branch work is based on this PR work. Because I need to make GUI modifications specifically for CMI, which means I need to have the newest wxGlade version first (i.e. this PR's work). So in this case, I can only wait for this PR to be merged first (i.e. the simplest way you said in email)? i.e. I cannot do git rebase afterwards anymore? Or I create the new branch (say branch gui_change_for_cmi for my next PR to the "new" branch) based on the current "maint" branch?

sorry I asked you once similar thing in the email, but want to make sure I understand the workflow correctly.

pavoljuhas commented 5 years ago

@dragonyanglong - I just merged this PR into the "maint" branch and propagated it to the "master" and "next" branches.

For your new CMI-only work you should start a new branch from the current diffpy/next branch and when ready, create a PR towards diffpy/next.

# assume "upstream" is https://github.com/diffpy/diffpy.pdfgui.git
git fetch upstream next
git checkout --no-track -b diffpy-cmi-engine FETCH_HEAD
# assume "long" is https://github.com/dragonyanglong/diffpy.pdfgui.git
git push -u long diffpy-cmi-engine

This should create a new "diffpy-cmi-engine" branch and set it up with an upstream in your pdfgui GitHub fork. (please pick a different name if more descriptive)

You can send me an email or call if something is not clear.

dragonyanglong commented 5 years ago

Got it. Thanks a lot, @pavoljuhas