Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove spaces in extras_require #49

Closed
cancan101 opened this issue Sep 27, 2017 · 5 comments
Closed

Remove spaces in extras_require #49

cancan101 opened this issue Sep 27, 2017 · 5 comments

Comments

@cancan101
Copy link
Collaborator

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
Copy link
Owner

loli commented Oct 1, 2017

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.

@loli loli closed this as completed Oct 1, 2017
@cancan101
Copy link
Collaborator Author

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
Copy link
Owner

loli commented Oct 1, 2017

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?

@loli loli reopened this Oct 1, 2017
@cancan101
Copy link
Collaborator Author

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
Copy link
Owner

loli commented Oct 4, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants