FIX: Fix or suppress the warning. #91
Conversation
pyproject.toml
Outdated
| "cython>=0.28.0", | ||
| "cython>=3", |
There was a problem hiding this comment.
Bump to Cython>=3.
There was a problem hiding this comment.
It sounds reasonable to me for now, but we may consider reverting this change if some people complain about dependencies. I am generally reluctant to tighten dependencies unless it's strongly necessary.
By the way, what's the change that indeed requires cython >= 3?
There was a problem hiding this comment.
https://github.com/r9y9/pyopenjtalk/pull/91/files#r1893870541
https://cython.readthedocs.io/en/3.0.x/src/userguide/migrating_to_cy30.html#numpy-c-api
Cython used to generate code that depended on the deprecated pre-NumPy-1.7 C-API. This is no longer the case with Cython 3.0.
Building with Cython<3 installed will fail.
There was a problem hiding this comment.
diff --git a/pyproject.toml b/pyproject.toml
index 0599f7a..0e27bb7 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -2,7 +2,7 @@
requires = [
"setuptools>=64",
"setuptools_scm>=8",
- "cython>=3",
+ "cython<3",
"cmake",
"numpy>=1.25.0; python_version>='3.9'",
"oldest-supported-numpy; python_version<'3.9'",
@@ -52,7 +52,7 @@ docs = [
"jupyter",
]
dev = [
- "cython>=3",
+ "cython<3",
"pysen",
"types-setuptools",
"mypy<=0.910",With this PR and above, it works OK in my environment
$ pip install -e . -vvv
...
g++ -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I/Users/ryuichi/anaconda3/envs/py38/include -arch x86_64 -I/Users/ryuichi/anaconda3/envs/py38/include -arch x86_64 -bundle -undefined dynamic_lookup build/temp.macosx-10.9-x86_64-cpython-38/lib/hts_engine_API/src/lib/HTS_audio.o build/temp.macosx-10.9-x86_64-cpython-38/lib/hts_engine_API/src/lib/HTS_engine.o build/temp.macosx-10.9-x86_64-cpython-38/lib/hts_engine_API/src/lib/HTS_gstream.o build/temp.macosx-10.9-x86_64-cpython-38/lib/hts_engine_API/src/lib/HTS_label.o build/temp.macosx-10.9-x86_64-cpython-38/lib/hts_engine_API/src/lib/HTS_misc.o build/temp.macosx-10.9-x86_64-cpython-38/lib/hts_engine_API/src/lib/HTS_model.o build/temp.macosx-10.9-x86_64-cpython-38/lib/hts_engine_API/src/lib/HTS_pstream.o build/temp.macosx-10.9-x86_64-cpython-38/lib/hts_engine_API/src/lib/HTS_sstream.o build/temp.macosx-10.9-x86_64-cpython-38/lib/hts_engine_API/src/lib/HTS_vocoder.o build/temp.macosx-10.9-x86_64-cpython-38/pyopenjtalk/htsengine.o -o build/lib.macosx-10.9-x86_64-cpython-38/pyopenjtalk/htsengine.cpython-38-darwin.so
copying build/lib.macosx-10.9-x86_64-cpython-38/pyopenjtalk/openjtalk.cpython-38-darwin.so -> pyopenjtalk
copying build/lib.macosx-10.9-x86_64-cpython-38/pyopenjtalk/htsengine.cpython-38-darwin.so -> pyopenjtalk
Creating /Users/ryuichi/anaconda3/envs/py38/lib/python3.8/site-packages/pyopenjtalk.egg-link (link to .)
Adding pyopenjtalk 0.3.5.dev10+ge01c0a9.d20241225 to easy-install.pth file
Installed /Users/ryuichi/Dropbox/sp/pyopenjtalk
Successfully installed pyopenjtalk-0.3.5.dev10+ge01c0a9.d20241225
Removed build tracker: '/private/var/folders/_z/zg2q619x6ql74chs2mm0wf9r0000gn/T/pip-req-tracker-2yy2p_0b'
As far as I understand, using cython <3 should be fine. Cython < 3 and newer numpy won't work though.
There was a problem hiding this comment.
I have also confirmed that it works with Cython<3.
Revert to cython>=0.28.0
There was a problem hiding this comment.
By the way, I noticed that Python 3.8 requires at least Cython 0.29.12.
The minimum version for which a wheel is provided is 0.29.14.
https://pypi.org/project/Cython/0.29.14/
The minimum version for which Programming Language::Python::3.8 exists in classifiers is 0.29.16.
https://pypi.org/project/Cython/0.29.16/
There was a problem hiding this comment.
Fixing requirements to "cython>=0.29.16" looks good to me!
There was a problem hiding this comment.
Set to "cython>=0.29.16"
| @@ -148,6 +163,7 @@ def check_cmake_in_path(): | |||
| language="c++", | |||
| define_macros=[ | |||
| ("AUDIO_PLAY_NONE", None), | |||
| ("NPY_NO_DEPRECATED_API", "NPY_1_7_API_VERSION"), | |||
There was a problem hiding this comment.
From Cython 3.0 onwards, defining NPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION will prevent the deprecated Numpy C API from being used.
https://cython.readthedocs.io/en/3.0.x/src/userguide/migrating_to_cy30.html#numpy-c-api
https://cython.readthedocs.io/en/latest/src/userguide/numpy_tutorial.html#compilation-using-setuptools
It is also required for the pyopenjtalk.openjtalk module, but it will be omitted in this PR because it will no longer be necessary in #87.
| @@ -1,6 +1,7 @@ | |||
| # coding: utf-8 | |||
| # cython: boundscheck=True, wraparound=True | |||
| # cython: c_string_type=unicode, c_string_encoding=ascii | |||
| # cython: language_level=3 | |||
There was a problem hiding this comment.
Cython 3 now warns about code that has no language-level.
| msvc_define_macros_config = [ | ||
| ("_CRT_NONSTDC_NO_WARNINGS", None), | ||
| ("_CRT_SECURE_NO_WARNINGS", None), | ||
| ] | ||
|
|
||
|
|
||
| def msvc_define_macros(macros): | ||
| mns = set([i[0] for i in macros]) | ||
| xs = filter(lambda x: x[0] not in mns, msvc_define_macros_config) | ||
| return list(chain(macros, xs)) |
There was a problem hiding this comment.
MSVC will warn for POSIX names.
It will also warn for functions that have alternatives in Annex K.
https://learn.microsoft.com/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-170#posix-function-names
https://learn.microsoft.com/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-170#unsafe-crt-library-functions
Suppresses the warning because it is impractical to write POSIX-compatible code without using these functions.
sabonerune
force-pushed
the
fix/fix-or-suppress-warning
branch
from
b5e7165 to
e01c0a9
Compare
| cdef class HTSEngine(object): | ||
| cdef class HTSEngine: |
There was a problem hiding this comment.
This can be made unnecessary by setting language_level=3.
https://cython.readthedocs.io/en/3.0.x/src/changes.html#id33
- With language_level=3/3str, Python classes without explicit base class are now new-style (type) classes also in Py2. Previously, they were created as old-style (non-type) classes. (Github issue #3530)
(It doesn't change anything since Python 3 isn't supported in the first place.)
pyproject.toml
Outdated
| "cython>=0.28.0", | ||
| "cython>=3", |
There was a problem hiding this comment.
It sounds reasonable to me for now, but we may consider reverting this change if some people complain about dependencies. I am generally reluctant to tighten dependencies unless it's strongly necessary.
By the way, what's the change that indeed requires cython >= 3?
Fixes or suppresses warnings that occur when building native code that can be resolved with pyopenjtalk.
This makes it easier to understand the cause of the error.
There are still a lot of warnings, but I think they should be fixed in the
OpenJTalk/HTSEnginerepository.