Skip to content

Make typing and typed-ast external dependencies #2452

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

Merged
merged 9 commits into from
Jan 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion MANIFEST.in
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
recursive-include lib-typing *.py
recursive-include scripts *
recursive-exclude scripts myunit
2 changes: 2 additions & 0 deletions build-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
setuptools
wheel
3 changes: 2 additions & 1 deletion mypy/test/testcmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from mypy.test.config import test_data_prefix, test_temp_dir
from mypy.test.data import parse_test_cases, DataDrivenTestCase
from mypy.test.helpers import assert_string_arrays_equal
from mypy.version import __version__
from mypy.version import __version__, base_version

# Path to Python 3 interpreter
python3_path = sys.executable
Expand Down Expand Up @@ -99,5 +99,6 @@ def normalize_file_output(content: List[str], current_abs_path: str) -> List[str
timestamp_regex = re.compile('\d{10}')
result = [x.replace(current_abs_path, '$PWD') for x in content]
result = [x.replace(__version__, '$VERSION') for x in result]
result = [x.replace(base_version, '$VERSION') for x in result]
result = [timestamp_regex.sub('$TIMESTAMP', x) for x in result]
return result
9 changes: 9 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,12 @@ parallel = true

[coverage:report]
show_missing = true

[metadata]
# This seems to be used only by bdist_wheel.
# You need "pip3 install wheel".
# Then run "python3 setup.py bdist_wheel" to build a wheel file
# (and then upload that to PyPI).
requires-dist =
typed-ast >= 0.6.3
typing >= 3.5.3; python_version < "3.5"
22 changes: 17 additions & 5 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
sys.stderr.write("ERROR: You need Python 3.2 or later to use mypy.\n")
exit(1)

from distutils.core import setup
from distutils.command.build_py import build_py
# This requires setuptools when building; setuptools is not needed
# when installing from a wheel file (though it is still neeeded for
# alternative forms of installing, as suggested by README.md).
from setuptools import setup
Copy link
Contributor

Choose a reason for hiding this comment

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

This causes $VERSION to be set as an environment variable, hence cobertura tests fail. I'm looking into how to fix this cleanly. Also, I don't quite understand why 3.6 tests passed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Really? By what mechanism does $VERSION get set?

Copy link
Contributor

@ambv ambv Dec 12, 2016

Choose a reason for hiding this comment

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

I am talking nonsense. What happens is as follows:

  • the XML report template in test-data/unit/cmdline.test has $VERSION and $TIMESTAMP abstracted away because they will obviously be different too often
  • the code responsible for it is in mypy/test/testcmdline.py, line 101
  • however, with setuptools the test runner's mypy.__version__ is "0.4.7-dev-2c9250eb182708402c66f8197f62ebe0190a43d5-dirty" so it doesn't replace the "0.4.7-dev" string in the XML

I'm investigating now why this is the case.

Copy link
Contributor

@ambv ambv Dec 12, 2016

Choose a reason for hiding this comment

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

The test harness does python setup.py install which is different on setuptools from python setup.py develop (which is the equivalent of pip install -e .).

There's two ways forward to fix this:

  1. Since local developers are likely going to keep using pip install -e . or equivalent, we can simply change the command that says python setup.py install to say python setup.py develop. I don't like this because this makes the cmdline test less isolated (we never perform a complete installation).
  2. Alternatively, let's just add line 102 to mypy/test/testcmdline.py which performs another replace to $VERSION, but this time using mypy.version.base_version and not mypy.version.__version__. This makes it work for both cases and won't break when we implement Version doesn't include git commit hash when installed via "pip3 install ." #2547.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I think about as far as I got the previous time around. The big mystery is why 3.6-dev behaves differently. I noticed that there was something different in the way lxml is installed. See previous comments (search for lxml).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm for (2). I'm not sure what pip install -e . or python setup.py develop do (the help text is kind of minimal) so (1) is particularly unattractive for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why setuptools would behave differently on 3.6, no idea. lxml 3.6.4 is a red herring, the difference is because there is a lxml 3.6.4 wheel pre-built for 3.5 on PyPI but none for 3.6. This went away BTW since there's just .tar.gz for lxml 3.7.0.

I can confirm both of my suggested fixes solve the failure.

from setuptools.command.build_py import build_py
from mypy.version import base_version
from mypy import git

Expand Down Expand Up @@ -81,18 +84,26 @@ def run(self):
'Programming Language :: Python :: 3.3',
'Programming Language :: Python :: 3.4',
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Topic :: Software Development',
]


package_dir = {'mypy': 'mypy'}
if sys.version_info < (3, 5, 0):
package_dir[''] = 'lib-typing/3.2'

scripts = ['scripts/mypy', 'scripts/stubgen']
if os.name == 'nt':
scripts.append('scripts/mypy.bat')

# These requirements are used when installing by other means than bdist_wheel.
# E.g. "pip3 install ." or
# "pip3 install git+git://github.com/python/mypy.git"
# (as suggested by README.md).
install_requires = []
install_requires.append('typed-ast >= 0.6.3')
if sys.version_info < (3, 5):
Copy link
Contributor

Choose a reason for hiding this comment

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

if/else statements like these produce incorrect wheels, e.g. dependent on the Python version with which setup.py was called. Instead, you should say:
extras_require={':python_version<"3.5"': ['typing >= 3.5.2']},
inside the setup() call.

Copy link
Member Author

Choose a reason for hiding this comment

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

How certain are you of that? IIRC I tested this and I found that the dependencies in the wheel are determined by what's in setup.cfg, not by the code here. But I am certainly mostly cargo-cult-coding here, since I haven't found decent docs on how to do these things. (Just StackOverflow questions, and various bits of doc that recommend things that don't work when I try them.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I got a report to that effect on my configparser backport: https://bitbucket.org/ambv/configparser/pull-requests/1/use-specifiers-to-indicate-dependence-on/diff

You might be right that setup.cfg is enough, I'll play with this to confirm/deny.

install_requires.append('typing >= 3.5.3')

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why this is here.

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean you don't understand why it was there right? mypy itself depends on the typing module. Previously mypy just installed a copy of typing.py (lib-typing/3.2/typing.py), but only for Python 3.4 and earlier, where it's not in the stdlib -- for Python 3.5 it depends on the stdlib (and installing another copy wouldn't have any effect anyway).

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, that explains it. I'm just used to py_modules being defined on the packages that implement a feature, not for dependencies.

setup(name='mypy-lang',
version=version,
description=description,
Expand All @@ -103,10 +114,11 @@ def run(self):
license='MIT License',
platforms=['POSIX'],
package_dir=package_dir,
py_modules=['typing'] if sys.version_info < (3, 5, 0) else [],
py_modules=[],
packages=['mypy'],
scripts=scripts,
data_files=data_files,
classifiers=classifiers,
cmdclass={'build_py': CustomPythonBuild},
install_requires=install_requires,
)
3 changes: 2 additions & 1 deletion test-data/unit/pythoneval.test
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ print('hello, world')
[out]
hello, world

[case testAbstractBaseClasses]
-- Skipped because different typing package versions have different repr()s.
[case testAbstractBaseClasses-skip]
import re
from typing import Sized, Sequence, Iterator, Iterable, Mapping, AbstractSet

Expand Down