Skip to content

Yet another PR to fix py38 #80

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

Merged
tacaswell merged 16 commits into from
Mar 26, 2020
Merged

Yet another PR to fix py38 #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
3770032
Run cythonize upon build (#52)
iamklob Oct 18, 2018
96cfd10
Fix bug: ld cannot find libdatrie
ashwinvis Jan 14, 2020
6d841a0
BLD: make cython mandatory at build time
tacaswell Mar 24, 2020
4c98b77
MNT: update trove classifier
tacaswell Mar 24, 2020
be7c376
BLD: remove the cython derived c files
tacaswell Mar 24, 2020
1c45182
Add pyproject.toml
ashwinvis Mar 11, 2020
db85206
Update appveyor (py37, py38), tox & travis (py38)
ashwinvis Jan 30, 2020
42ec8bb
TST: add cython to tox dependencies
tacaswell Mar 25, 2020
1015be1
CI: install cython before running tox
tacaswell Mar 25, 2020
9a2e768
MNT: account for collection.abc move
tacaswell Mar 25, 2020
78e5e15
DOC: add release notes for 0.8.2
tacaswell Mar 25, 2020
7ede3fe
DOC: add section for 0.8.1
tacaswell Mar 25, 2020
aa68911
BLD: explicitly set the cython language level
tacaswell Mar 25, 2020
3661277
BLD: bump version in setup.py
tacaswell Mar 26, 2020
bc23188
MNT: ignore wheelhouse directory
tacaswell Mar 26, 2020
341d910
BLD: add scripts for using manylinux docker
tacaswell Mar 26, 2020
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
5 changes: 5 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,8 @@ dist/

*.so
build/

src/datrie.c
src/cdatrie.c
src/stdio_ext.c
wheelhouse
3 changes: 2 additions & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ python:
- "3.5"
- "3.6"
- "3.7"
- "3.8"

install:
- pip install tox-travis
- pip install tox-travis cython
Copy link
Contributor

@ashwinvis ashwinvis Mar 25, 2020

Choose a reason for hiding this comment

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

Why? Shouldn't pyproject.toml take care of it?

Copy link
Contributor Author

@tacaswell tacaswell Mar 25, 2020

Choose a reason for hiding this comment

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

Experimentally it did not with the version of pip I was testing with locally. I don't think that the pyproject.toml work is settled enough to rely on widely yet.

Copy link
Contributor

@ashwinvis ashwinvis Mar 25, 2020

Choose a reason for hiding this comment

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

This was the reason that I kept the C sources... so that legacy applications which might depend on old pip / setuptools might have a chance to use datrie. I guess you want an explicit ImportError of Cython instead, then?

Copy link
Contributor

@ashwinvis ashwinvis Mar 25, 2020

Choose a reason for hiding this comment

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

If you really want to go this way I would request you to bump the version to 0.9.0 - because it is a breaking change not a bug fix (think, semver).

Copy link
Contributor Author

@tacaswell tacaswell Mar 25, 2020

Choose a reason for hiding this comment

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

It is also possible to run cython as part of the release process and still include the c-files in the sdist, however that just seems like a trap for user.

The better solution is to provide as many wheels as we can so that most users don't even have to have a compiler.

scikit-image is on a "must have cython to install from source" model as well.

Copy link
Contributor

@ashwinvis ashwinvis Mar 25, 2020

Choose a reason for hiding this comment

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

Wheels sound like a good idea, but may not be so easy.

Copy link
Contributor Author

@tacaswell tacaswell Mar 25, 2020

Choose a reason for hiding this comment

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

I think I know how to do the linux ones, it looks like CGohlke is already building the windows ones (we just need to upload), the mac ones are the only problem. Would prefer to find a mac-developer to build them.

I disagree that this is a breaking change, packaging / installing is not part of the API.

Copy link
Contributor

@ashwinvis ashwinvis Mar 25, 2020

Choose a reason for hiding this comment

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

You have a point. I am OK with this PR then.


script: tox
12 changes: 12 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
CHANGES
=======

0.8.2 (2020-03-25)
------------------
* Future-proof Python support by making cython a build time dependency and
removing cython generated c files from the repo (and sdist).
* Fix collections.abc.MutableMapping import
* CI and test updates
* Adjust library name to unbreak some linkers

0.8.1 (skipped)
---------------
This version intentionally skipped

0.8 (2019-07-03)
----------------
* Python 3.7 compatibility; extension is rebuilt with Cython 0.29.11.
Expand Down
16 changes: 16 additions & 0 deletions appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,22 @@ environment:
PYTHON_VERSION: "3.6.x"
PYTHON_ARCH: "64"

- PYTHON: "C:\\Python37"
PYTHON_VERSION: "3.7.x"
PYTHON_ARCH: "32"

- PYTHON: "C:\\Python37-x64"
PYTHON_VERSION: "3.7.x"
PYTHON_ARCH: "64"

- PYTHON: "C:\\Python38"
PYTHON_VERSION: "3.8.x"
PYTHON_ARCH: "32"

- PYTHON: "C:\\Python38-x64"
PYTHON_VERSION: "3.8.x"
PYTHON_ARCH: "64"

install:
# Install Python (from the official .msi of https://www.python.org/) and pip when
# not already installed.
Expand Down
3 changes: 3 additions & 0 deletions dev-requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
cython
hypothesis
pytest
6 changes: 6 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[build-system]
requires = [
"setuptools>=40.8.0",
"wheel",
"Cython"
]
26 changes: 16 additions & 10 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

from setuptools import setup, Extension

from Cython.Build import cythonize

LIBDATRIE_DIR = 'libdatrie'
LIBDATRIE_FILES = sorted(glob.glob(os.path.join(LIBDATRIE_DIR, "datrie", "*.c")))

Expand All @@ -27,32 +29,36 @@
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: Implementation :: CPython',
'Topic :: Software Development :: Libraries :: Python Modules',
'Topic :: Scientific/Engineering :: Information Analysis',
'Topic :: Text Processing :: Linguistic'
]

ext_modules = cythonize(
'src/datrie.pyx', 'src/cdatrie.pxd', 'src/stdio_ext.pxd',
annotate=True,
include_path=[os.path.join(os.path.dirname(os.path.abspath(__file__)), "src")],
language_level=2
)

for m in ext_modules:
m.include_dirs=[LIBDATRIE_DIR]

setup(name="datrie",
version="0.8",
version="0.8.2",
description=DESCRIPTION,
long_description=LONG_DESCRIPTION,
author='Mikhail Korobov',
author_email='kmike84@gmail.com',
license=LICENSE,
url='https://github.com/kmike/datrie',
classifiers=CLASSIFIERS,
libraries=[('libdatrie', {
libraries=[('datrie', {
"sources": LIBDATRIE_FILES,
"include_dirs": [LIBDATRIE_DIR]})],
ext_modules=[
Extension("datrie", [
'src/datrie.c',
'src/cdatrie.c',
'src/stdio_ext.c'
], include_dirs=[LIBDATRIE_DIR])
],

ext_modules=ext_modules,
python_requires=">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*",
setup_requires=["pytest-runner", 'Cython>=0.28'],
tests_require=["pytest", "hypothesis"])
Loading