bastula / dicompyler

Extensible radiation therapy research platform and viewer for DICOM and DICOM RT
http://www.dicompyler.com
263 stars 99 forks source link

Better fix for #122 to allow for more recent matplotlib #142

Closed jmartens closed 1 year ago

jmartens commented 2 years ago

Use matplotlib/legacycontour in order to redo #122 with minimal changes and being able to use more recent/current matplotlib versions

sjswerdloff commented 2 years ago

It seems like appveyor builds have been broken for a long time. I looked through the history, and none of them succeeded... it went back to the 1.25 build based on @bastula pinning matplotlib (the lesser fix ? ) Should there be an appveyor.yml file ? Is it acceptable for the appveyor builds to be broken (my guess is appveyor must have changed in some way at some point, and what worked before doesn't work anymore... but it's a wild guess).

bastula commented 2 years ago

@sjswerdloff AppVeyor builds haven't worked. It was something I was toying with at the time, but never put time into.

@jmartens I think this is a decent stopgap solution for Linux/Mac without completely re-implementing with scikit-image. Do you have any proposals on how this should be installed on Windows without a C compiler (and/or Anaconda)?

sjswerdloff commented 2 years ago

@sjswerdloff AppVeyor builds haven't worked. It was something I was toying with at the time, but never put time into.

Then I encourage removing appveyor from your CI... I feel a lot more comfortable reviewing code or changes that have passed tests...

jmartens commented 2 years ago

@jmartens I think this is a decent stopgap solution for Linux/Mac without completely re-implementing with scikit-image. Do you have any proposals on how this should be installed on Windows without a C compiler (and/or Anaconda)?

I cam up with this fix on Windows and it seems to work fine there, so don't see any issue here:

C:\>python -m venv %TEMP%\venv-legacycontour

C:\> %TEMP%\venv-legacycontour\Scripts\activate

(venv-legacycontour) C:\>python -c "from legacycontour import _cntr as contour"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'legacycontour'

So not found in a clean installed virtual environment (as expected).

Now install it:

(venv-legacycontour) C:\>pip install git+https://github.com/matplotlib/legacycontour.git
Collecting git+https://github.com/matplotlib/legacycontour.git
  Cloning https://github.com/matplotlib/legacycontour.git to c:\users\jonathan.martens\appdata\local\temp\pip-req-build-tcj7idd1
  Running command git clone --filter=blob:none -q https://github.com/matplotlib/legacycontour.git 'C:\Users\*******\AppData\Local\Temp\pip-req-build-tcj7idd1'
  Resolved https://github.com/matplotlib/legacycontour.git to commit 4b2ad15dbcbe730db9e0e3692dfde06fd3ebcd69
  Preparing metadata (setup.py) ... done
Collecting numpy>=1.7.1
  Using cached numpy-1.21.3-cp310-cp310-win_amd64.whl (14.0 MB)
Collecting matplotlib>=1.5
  Using cached matplotlib-3.4.3.tar.gz (37.9 MB)
  Preparing metadata (setup.py) ... done
Collecting cycler>=0.10
  Using cached cycler-0.11.0-py3-none-any.whl (6.4 kB)
Collecting kiwisolver>=1.0.1
  Using cached kiwisolver-1.3.2-cp310-cp310-win_amd64.whl (52 kB)
Collecting pillow>=6.2.0
  Using cached Pillow-8.4.0-cp310-cp310-win_amd64.whl (3.2 MB)
Collecting pyparsing>=2.2.1
  Using cached pyparsing-3.0.4-py3-none-any.whl (96 kB)
Collecting python-dateutil>=2.7
  Using cached python_dateutil-2.8.2-py2.py3-none-any.whl (247 kB)
Collecting six>=1.5
  Using cached six-1.16.0-py2.py3-none-any.whl (11 kB)
Using legacy 'setup.py install' for legacycontour, since package 'wheel' is not installed.
Using legacy 'setup.py install' for matplotlib, since package 'wheel' is not installed.
Installing collected packages: six, python-dateutil, pyparsing, pillow, numpy, kiwisolver, cycler, matplotlib, legacycontour
    Running setup.py install for matplotlib ... done
    Running setup.py install for legacycontour ... done
Successfully installed cycler-0.11.0 kiwisolver-1.3.2 legacycontour-0+untagged.4.g4b2ad15 matplotlib-3.4.3 numpy-1.21.3 pillow-8.4.0 pyparsing-3.0.4 python-dateutil-2.8.2 six-1.16.0

(venv-legacycontour) C:\>

And try again (after install):

(venv-legacycontour) C:\>python -c "from legacycontour import _cntr as contour"

(venv-legacycontour) C:\>

Since it is installed properly, no import error anymore.

jmartens commented 2 years ago

Overlooked this comment:

Do you have any proposals on how this should be installed on Windows without a C compiler (and/or Anaconda)?

It might have worked on my end since I do have a C compiler installed. Not sure if we can do without as I am unsure of matplotlib will install without a C compiler on Windows as well.

Perhaps we should work to get legacycontour onto pypi as well. Perhaps @WeatherGod is willing to do so.