openworm / tracker-commons

Compilation of information and code bases related to open-source trackers for C. elegans
11 stars 12 forks source link

Python wcon pip package dependencies not enforced? #168

Open cheelee opened 6 years ago

cheelee commented 6 years ago

Tried using the wcon pip package directly to process WCON files, but it seems like the python packages wcon depends on aren't also loaded by default. Is that true?

JimHokanson commented 6 years ago

See: https://github.com/openworm/tracker-commons/blob/master/src/Python/setup.py

I think the answer to your question is yes and that this behavior probably needs to be fixed.

cheelee commented 6 years ago

Cool, I'll give it a shot and see if I can get it going. Thanks!

cheelee commented 6 years ago

Ok I see @MichaelCurrie 's reasoning ... but I think this leaves the package vulnerable to incompatible versions of numpy/scipy. My current setup inadvertently has numpy/scipy already installed via other dependencies, and is complaining of:

File "/Volumes/OpenWorm/repository/git/sandbox/nwb-explorer/env/lib/python3.6/site-packages/wcon/measurement_unit.py", line 24, in from scipy.constants import F2C, K2C, C2F, C2K ImportError: cannot import name 'F2C'

Personally I'd rather have the package report that it requires a version of numpy/scipy that conflicts with the current one, than silently succeed.

Addendum: It would appear scipy has deprecated temperature conversion from 0.18 onwards: https://docs.scipy.org/doc/scipy-0.19.0/reference/generated/scipy.constants.convert_temperature.html#scipy.constants.convert_temperature

cheelee commented 4 years ago

Finally got my butt back to systematically looking at this after a very long hiatus.

Current state of our code base:

  1. Uses F2C which was last supported by scipy 0.17.1;
  2. scipy 0.17.1 was last supported by python 3.5 (breaks under the latest 3.7)

Idea and further action:

  1. Retest the current master package under the old environments (Python 2.7 + Python 3.5 with scipy 0.17.1)
  2. If all goes well, tag (and backdate?) that as a release with the above dependencies enforced.
  3. Fix the F2C deprecation issue (see above) for a modernized release compatible with Python 3.7. Also consider dropping support altogether for Python 2.7 which is about to be deprecated Jan 2020.
MichaelCurrie commented 4 years ago

I agree with testing the code again to make sure it works, good idea.

Removing support for 2.7 probably won't be of significant benefit, in my opinion, since the code is already set up to support both 2.7 and 3.x, so it's not like there are two codebases to maintain. Probably not worth the effort, and might even be a negative for some forlorn individuals still clinging to 2.7 :)

Thanks @cheelee

cheelee commented 4 years ago

@MichaelCurrie I was able to get the tests running again. The existing code base made use of two features that were deprecated by scipy after 0.17.1 and pandas after 0.19.0. After I had successfully made the test scripts use the older packages, our tests were able to finally succeed. There is still one deprecation warning from the way we are using jsonschema for the Python 3+ versions however.

I was compelled to stop using miniconda, and to get the specific packages directly using pip. Specifically miniconda refuses to locate scipy 0.17.x.

Was there a reason we chose to handle the package dependencies using miniconda? If not, do you think it safe to switch back to using pip & requirements.txt? So far the latter has felt far more reliable a source of packages than miniconda.