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.

Sign up for GitHub

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

Jump to bottom

Don't import appdirs from setup.py #92

Merged
merged 1 commit into from Apr 29, 2017
Merged

Don't import appdirs from setup.py #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions appdirs.py
View file
Open in desktop
Expand Up @@ -13,8 +13,8 @@
# - Mac OS X: http://developer.apple.com/documentation/MacOSX/Conceptual/BPFileSystem/index.html
# - XDG spec for Un*x: http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html

__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("."))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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('.'))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!



import sys
Expand Down
11 changes: 9 additions & 2 deletions setup.py
View file
Open in desktop
Expand Up @@ -7,7 +7,7 @@
from setuptools import setup
except ImportError:
from distutils.core import setup
import appdirs
import ast

tests_require = []
if sys.version_info < (2, 7):
Expand All @@ -21,9 +21,16 @@ def read(fname):
return out


# Do not import `appdirs` yet, lest we import some random version on sys.path.
for line in read("appdirs.py").splitlines():
if line.startswith("__version__"):
version = ast.literal_eval(line.split("=", 1)[1].strip())
break


setup(
name='appdirs',
version=appdirs.__version__,
version=version,
description='A small Python module for determining appropriate ' + \
'platform-specific dirs, e.g. a "user data dir".',
long_description=read('README.rst') + '\n' + read('CHANGES.rst'),
Expand Down