vsoch / scif

scientific filesystem: a filesystem organization for scientific software and metadata
https://sci-f.github.io/
Mozilla Public License 2.0
30 stars 13 forks source link

inadvertent Python 3.6 dependency? #63

Closed samcmill closed 4 years ago

samcmill commented 4 years ago

scif test ref produces:

  File "/usr/local/lib/python3.5/dist-packages/scif/utils/terminal.py", line 47, in run_command
    process = Popen(cmd, stderr=STDOUT, stdout=PIPE, encoding="utf8")
TypeError: __init__() got an unexpected keyword argument 'encoding'

The encoding option was added in Python version 3.6. (The above error was generated using Python 2.7 and 3.5.)

I'm assuming this was an inadvertent usage since setup.py claims Python 2 compatibility?

If the encoding option is removed, then the condition needs to be modified to use if output == b"" rather than if output == ""

vsoch commented 4 years ago

I've always had (and tested with) 3.7.x or greater, so I haven't hit these issues. It's a mistake that setup.py includes Python 2 compatibility (I should have removed it).

Would you care to open a PR for these small changes? I'll update the setup.py.

samcmill commented 4 years ago

Just to clarify, the intent is to support Python 3.x, including pre-3.6, but not 2.x?

vsoch commented 4 years ago

Any reason we can't just change the setup to require 3.6 or up?

            "Programming Language :: Python :: 3.6",
vsoch commented 4 years ago

What user or container bases would not be supported to do Python 3.6 or Python 3.7 and up? We are already at 3.9 and I suspect we will be hitting 4's in 2020.

samcmill commented 4 years ago

That's your call.

The Python 3 that ships with Ubuntu 16.04 is 3.5.

vsoch commented 4 years ago

Hmm, so the version issue already exists in the released scif (meaning built containers, likely not many) and the question is if 16.04 should be supported moving forward. I think Ubuntu 16.04 already hit end of life, so supporting it for new container builds wouldn't make much sense.

I'm running Ubuntu 18.04 and my base Python is 3.6

$ /usr/bin/python3 --version
Python 3.6.9

I would suggest based on Ubuntu 16.04 not being supported, we would be better to encourage users to build using 18.04 anyway, and should support 3.6 and up. And of course moving forward we have better awareness of this (I haven't had an issue open about it). What do you think?

samcmill commented 4 years ago

The particular case where I hit this was building on top of a pre-existing compiler container image that was in turn based on 16.04. I could, of course, generate my own version of the compiler base image based on 18.04, but then I'd be operating in unsupported waters.

For some period of time, there will continue to be third-party container images based on 16.04 (or similar vintage for other distributions) that users will want to built on top of.

I have no reservations about dropping 2.x support. But in general I recommend supporting 3.x, including pre-3.6. I have not done an exhaustive search for other 3.6-ism's, but if this is the only one then it seems unreasonable to me to cut off support for pre-3.6 since it's easy enough to fix up to support all Python 3.x versions.

vsoch commented 4 years ago

I think that's very sound logic, let's aim to support 3.x. I'll give a shot at the change you suggested, and I'll ping you when it's ready for review.

vsoch commented 4 years ago

okay, just updated the PR linked previously, #64. Would you care to take a look?

vsoch commented 4 years ago

Are you using scif for something cool? If we need to add additional tests, or if I can help, I'd really enjoy it! Let me know how I can help, beyond the issue here.

samcmill commented 4 years ago

I don't know whether it's cool or not, but I'm exploring approaches for containerizing optimized binaries while also running on a broad range of hardware. My ideal approach is "fat" binaries but unfortunately that doesn't work in all cases. An alternative is to

  1. Install multiple binaries in a single container image, e.g., optimized for different vector ISAs, and
  2. Use the "best" binary at container runtime based on the detected hardware

scif addresses # 1, but still exploring # 2.

vsoch commented 4 years ago

I think that's a pretty cool project! If I can help, I hope that you'll ask?

Does #64 fix the versioning issue?

vsoch commented 4 years ago

All set! We no longer support Python 2.x, but do support 3.x https://pypi.org/project/scif/0.0.80/.