loli / medpy

Medical image processing in Python
http://loli.github.io/medpy/
GNU General Public License v3.0
571 stars 138 forks source link

Remove spaces in `extras_require` #49

Closed cancan101 closed 6 years ago

cancan101 commented 7 years ago

Right now the extras_require is:

          extras_require = {
            'Additional image formats' : ["itk >= 3.16.0"]
          },

which contains a space in the key, something I have never seen before. This causes problems when trying to pip install. Perhaps there is a way to escape, but this seems non intuitive:

$ pip install "medpy[Additional image formats]"
...
pip._vendor.packaging.requirements.InvalidRequirement: Invalid requirement, parse error at "'[Additio'"

I suggest offering a string without a space.

loli commented 7 years ago

Apparently, using spaces in dictionary keys is absolutely valid, as discussed in https://stackoverflow.com/questions/13474957/spaces-in-python-dictionary-keys. But I can see your point that it becomes inconvenient in this case.

On the other hand, the itk package can't be installed via pip. Please see the documentation (https://loli.github.io/medpy/) for detailed instructions on how to compile ITK with Python wrappers.

cancan101 commented 7 years ago

I wasn't referring to spaces in dictionary keys in general, I was specifically referring to extra requires. Also I think there is now a wheel for itk so I think It can be pip installed.

loli commented 7 years ago

You're right, that's amazing. Thank you for pointing this out to me!

https://blog.kitware.com/itk-is-on-pypi-pip-install-itk-is-here/

Did you already tried if the medpy IO with that version of ITK?

cancan101 commented 7 years ago

Ok given that people probably were not using that extra (and that It didn't work until recently) what do you think about renaming it to all or extra_formats or itk?

loli commented 7 years ago

I would go with extra_formats, since that's the sole purpose of the itk module in MedPy. If you want, you can make the adjustment and create a pull request. Otherwise I'll fix it when I get around to it.