Don't import appdirs from setup.py #92
Conversation
In general, setup.py should never import the code it's trying to install. During a setup.py run, there is no guarantee that importing `appdirs` will actually import the one in the same directory as `setup.py`. Instead, read the version number out of `appdirs.py` by opening it.
| __version_info__ = (1, 4, 3) | ||
| __version__ = '.'.join(map(str, __version_info__)) | ||
| __version__ = "1.4.3" | ||
| __version_info__ = tuple(int(segment) for segment in __version__.split(".")) |
There was a problem hiding this comment.
This will fail if the version number has non-integers in it, like 'dev' or 'alpha' markers. I don't know if this is ever the case for this package. This may be an option then:
__version__ = "1.4.3.dev0" # as example
def intify(value):
try:
return int(value)
except ValueError:
return value
__version_info__ = map(intify, __version__.split('.'))
There was a problem hiding this comment.
Hi @mauritsvanrees!
Indeed the code is expecting integer version segments only.
I went thru all the history of this line and __version_info__ has always been a triple of integers.
In any case, whoever breaks the integer segments expectation will have a very early, reasonably obvious and clear error message in their hands, so I'm tempted to say YAGNI.
But I'm happy to defer to the maintainers on this.
There was a problem hiding this comment.
The module is so small, that we won't do anything other than version numbers. I appreciate the suggested addition though. I will merge the original change in tonight.
Cheers guys!
In general, setup.py should never import the code it's trying to
install.
During a setup.py run, there is no guarantee that importing
appdirswill actually import the one in the same directory as
setup.py.Instead, read the version number out of
appdirs.pyby opening it.Fixes: #91