pearu/pylibtiff

optparse_gui cannot be imported correctly on Windows

DManowitz opened this issue · 6 comments

optparse_gui.py attempts to import a module called killableprocess from the package, but this file does not exist. It seems that it was unneeded, as subprocess should work on Windows.

I can't speak for @pearu, but I think this module should be removed entirely and no longer supported by pylibtiff. optparse is deprecated and argparse should be used instead. My guess the missing file is needed on Windows to reliably kill sub-processes. See this file which appears to be equivalent: https://github.com/andreisavu/python-process/blob/master/killableprocess.py

@DManowitz What do you or did you want to use the optparse_gui.py module for?

I don't actually need it. I was mostly just playing around with things. If you want to kill off the file altogether, I have no problem with that.

So doing a little more code skimming, I found that this optparser is used in the command line scripts in pylibtiff. For example,

try:
from libtiff.optparse_gui import OptionParser
except ImportError:
from optparse import OptionParser

I see the main advantage being that since these command line scripts are given files, you can use a file dialog to choose the file you want to run the script on rather than providing it via the command line. My opinion is that this isn't needed anymore and that I personally don't want to maintain it. If @pearu still wants it then maybe we could remove the killableprocess usage and hope that it just works on Windows.

pearu commented

I agree that the usage of optparse/killableprocess could be updated/eliminated.

I am not sure how many users actually use these scripts. Originally, I implemented these only for my convenience. Therefore, I suggest doing that in a user-friendly way, that is, (i) increase the micro version bit and emit a deprecation warning and then (ii) increase the minor version bit and do the final goal. (ii) is a minimum required action.

The scripts can stick around, but the Wx-based GUI option stuff I think never worked on Windows anyway so I think it'd be safe to remove that GUI functionality from a minor version release.