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

Add PyPy to the build matrix and adapt for pypy pybind11 issue #61

Merged
merged 4 commits into from
Nov 18, 2021
Merged

Add PyPy to the build matrix and adapt for pypy pybind11 issue #61

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb44788
skip printing value to stdout on PyPy
mattip Nov 18, 2021
fb50f21
add builds for pypy
mattip Nov 18, 2021
53184c2
fixes from review, linter
mattip Nov 18, 2021
bc4d4ca
fix linter
mattip Nov 18, 2021
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
4 changes: 2 additions & 2 deletions .github/workflows/dists.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ jobs:

- name: Build wheels
env:
# Skip building on Python 2.7 and PyPy
# Skip building on Python 2.7
# Additionally, skip 32-bit Windows for now as MSVC needs seperate setup with different toolchain to do this
# Refer: https://cibuildwheel.readthedocs.io/en/stable/cpp_standards/#windows-and-python-27
CIBW_SKIP: cp27-* pp* *-win32 cp35-*
CIBW_SKIP: cp27-* *-win32 cp35-*
Copy link

Choose a reason for hiding this comment

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

You can remove the skipping of cp27 & cp35 since cibuildwheel 2.0 (in fact, it should be printing a warning stating these are not needed).

Copy link
Owner

Choose a reason for hiding this comment

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

yeah that's for the suggestion, but I think we intentionally skipped cp27, because the underlying C++ library won't compile and python2.7 is reaching end of life. But I cannot remember clearly why we skipped cp35 though. But I am pretty sure certain it was because an issue, though I am not sure what the issue was. Do you have a need for these two to work? If so I can take a look later probably.

Copy link

Choose a reason for hiding this comment

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

Cibuildwheel 2.0 dropped Python 2.7 and Python 3.5, so there's no need to skip them anymore, is what I meant. :)

Copy link
Owner

Choose a reason for hiding this comment

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

Ah make sense! Thanks for letting me knnow.

CIBW_BEFORE_TEST: pip install -r tests/requirements.txt
CIBW_TEST_COMMAND: pytest {project}/tests
CIBW_ARCHS_MACOS: "x86_64 universal2 arm64" # build on M1 chip
Expand Down
8 changes: 8 additions & 0 deletions src/encoding_decoding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,15 @@ toml::array py_list_to_toml_array(const py::list &list) {
arr.push_back(time_value);
} else {
std::stringstream ss;
#ifdef PYPY_VERSION
// see
bobfang1992 marked this conversation as resolved.
Show resolved Hide resolved
// https://github.com/conda-forge/pytomlpp-feedstock/pull/1#issuecomment-972738986
// and
// https://github.com/pybind/pybind11/issues/3408#issuecomment-972752210
ss << "not a valid type for conversion " << std::endl;
#else
ss << "not a valid type for conversion " << it << std::endl;
#endif
throw py::type_error(ss.str());
}
}
Expand Down
2 changes: 2 additions & 0 deletions tests/python-tests/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ class A:
pass
with pytest.raises(TypeError):
pytomlpp.dumps({'a': A()})
with pytest.raises(TypeError):
pytomlpp.dumps({'a': [A()]})

@pytest.mark.parametrize("toml_file", valid_toml_files)
def test_decode_encode_binary(toml_file, tmp_path):
Expand Down