Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to wxPython4 #170

Closed
Anthchirp opened this issue Apr 12, 2018 · 18 comments
Closed

Upgrade to wxPython4 #170

Anthchirp opened this issue Apr 12, 2018 · 18 comments
Assignees

Comments

@Anthchirp
Copy link
Member

Anthchirp commented Apr 12, 2018

As discussed in meeting 2018-04-12

@Anthchirp Anthchirp created this issue from a note in Python 2/3 Compatibility (Investigations) Apr 12, 2018
@Anthchirp Anthchirp moved this from Investigations to To do in Python 2/3 Compatibility Apr 12, 2018
@phyy-nx
Copy link
Contributor

phyy-nx commented Apr 26, 2018

Issue is to fix interface changes moving from wxPython3 to 4. Bootstrap now automatically pulls in wxPython4 if --python3 is specified.

@ndevenish
Copy link
Contributor

I made a start of an 3/4 compatible attempt at this in October ndevenish/cctbx_project@2f6abba .

As long as you don't need compatibility for both, a lot of this level of stuff could probably be done automatically.

IIRC this was enough to get the viewer mostly running (certainly syntax-correct), but had a crash opening the mask viewer that I didn't follow up on as part of this investigation.

@Anthchirp
Copy link
Member Author

Anthchirp commented Apr 26, 2018

List of applications that are affected:

cctbx_project

  • wxtbx
  • cctbx image viewer
  • crys3d
  • gltbx viewer
  • iota
  • mmtbx
  • prime
  • scitbx/rigid_body
  • simage (diffraction pattern simulator)
  • simtbx
  • xfel GUI
  • miscellaneous xfel apps

DIALS

  • DIALS geometry viewer
  • DIALS image viewer
  • DIALS multiplicity viewer
  • DIALS reciprocal lattice viewer
  • DIALS reflection table viewer
  • xia2

Phenix

  • Phenix image viewer
  • Phenix itself

@Anthchirp
Copy link
Member Author

Anthchirp commented Apr 26, 2018

Could everyone please look into wxPython3->4 migration and start collecting (not necessarily: committing) patches for migration in a separate branch until the 31st of August 2018.

@bkpoon
Copy link
Member

bkpoon commented Aug 24, 2018

Unfortunately, the current installation of wxPython4 via bootstrap.py does not work on CentOS 6. Installing dependencies as binary wheels with pip on linux is generally not portable unless the dependency is pure Python or is built according to PEP 513 (https://www.python.org/dev/peps/pep-0513/) (manylinux1, CentOS 5) or PEP 571 (https://www.python.org/dev/peps/pep-0571/) (manylinux2010, CentOS 6). And this really only applies to packages with no external dependencies (e.g. libpng or libhdf5) because the package should not link to versioned dependencies that can be different among different linux distributions. That is why we compile libpng, libtiff, etc. in install_base_packages.py. To be portable, wxPython4 should be built to link against the other dependencies built in install_base_packages.py, similar to how we build wxPython3. The wxPython website describes these issues here (https://wxpython.org/pages/downloads/), here (https://wiki.wxpython.org/How%20to%20install%20wxPython), and here (https://wxpython.org/blog/2017-08-17-builds-for-linux-with-pip/index.html). Windows and macOS should be fine, though.

To save effort on building (and probably adding more dependencies to satisfy wxPython4 prerequisites) and testing wxPython4 on the different Linux distributions we support, I suggest we wait until the migration to conda is done.

@Anthchirp
Copy link
Member Author

Yes, wxPython 4.0.1 having issues with CentOS 6 is a known thing. @ndevenish reported this back in March at wxWidgets/Phoenix#780.
To my knowledge this has been fixed with 4.0.2 upstream, but the commit allowing us to use that fix (7914fa8) is currently sitting in pull request #229 which is waiting (at a minimum) for the Phenix release.

Did you test with 4.0.1 or with a newer version?

@bkpoon
Copy link
Member

bkpoon commented Aug 24, 2018

I tried 4.0.3 after you updated the package-updates branch (#229) and with a separate Python installation independent of CCTBX. But the point remains that there is no portable binary build of wxPython4. Are you suggesting we build different installers for each linux distribution?

The goal of install_base_packages.py is to provide a single installation that is generally compatible with most linux distributions (the Phenix installers do work on Ubuntu 18.04 and general users can compile/link Rosetta to our CentOS 6 build with an appropriate flag to handle an ABI change). As was discussed at the GRC, the migration to conda for dependencies maintains that behavior while adding a single compiler target for linux that supports C++11, and making CCTBX-based projects generally more compatible with other linux distributions.

@Anthchirp
Copy link
Member Author

So what you are saying is that we can't use wheels and then use the result to build installers, right?

@ndevenish
Copy link
Contributor

I'm not sure why bootstrap is an issue here as you are going to be 100% dependent on Conda within two months? And this doesn't stop us/you using wxPython for development builds...

@bkpoon
Copy link
Member

bkpoon commented Aug 24, 2018

@Anthchirp No, as described above, wheels can generally be used, but not if they link to system-specific dependencies.

@ndevenish Bootstrap will be used until conda is deployed, so developers can use the pip version of wxPython4 for development and testing on their macs, windows, and the linux distributions that have binary wheels for wxPython4.

It does not look like wxPython4 syntax is backwards compatible with wxPython3 syntax (https://wxpython.org/Phoenix/docs/html/classic_vs_phoenix.html#classic-vs-phoenix) unless the GUI was written from the beginning to be compatible to both. So I would prefer that we just make the jump from wxPython3 directly to wxPython4 without having to build a compatibility layer for the 2 versions. This would require moving the 8/31 deadline to a later date to be determined, and having at least the majority of GUI projects be already converted before we make the hard switch

@Anthchirp
Copy link
Member Author

The existing proposal is to not have a compatibility layer or a migration timeframe, but a hard switch, which is why I previously said (#170 (comment)) to collect commits in a branch.
I agree that looking at the progress this may not be realistic for the end of August, especially since the phenix release has priority over this.

@ndevenish
Copy link
Contributor

Right, from that comment above and what I remember of the April 26th meeting we discussed this and the conclusion all round was that was exactly what we'd do - we decided building a compatibility layer wouldn't be worth the effort.

FWIW all the Dials GUI's are basically ready and will be by the 31st, the incompatible syntax is pretty minimal but the hard part is that the abstraction behaviour in wxPhoenix is more consistent with wxWidgets, so calls that silently did "extra" behaviour to e.g. silently retain objects, now don't. These were the hard problems to track down.

Given the two-month unexpected delay on the Phenix release I agree that the arbitrary deadline we agreed on probably isn't realistic, especially since I imagine most of the next two months will be spent switching to Conda?

@bkpoon
Copy link
Member

bkpoon commented Aug 24, 2018

Yep, that's the plan, along with things for Python 3 migration.

@graeme-winter
Copy link
Contributor

I guess we need to add a new issue flag "hold for conda" or something similar?

@graeme-winter graeme-winter added the hold for conda Held up by move to conda for base label Oct 17, 2018
@graeme-winter graeme-winter removed the hold for conda Held up by move to conda for base label May 13, 2019
@graeme-winter
Copy link
Contributor

Conda is now a thing => removed label.

@rjgildea
Copy link
Contributor

More than 40% of macOS users are on Mojave (which seemingly can't build wxpython3):
http://gs.statcounter.com/os-version-market-share/macos/desktop/worldwide

@Anthchirp
Copy link
Member Author

Dials has now moved to wxPython4 exclusively, so this is resolved for us.

Python 2/3 Compatibility automation moved this from To do to Completed Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

6 participants