Skip to content

Conversation

@tobwen
Copy link
Contributor

@tobwen tobwen commented Feb 8, 2026

summary

These changes fix type mismatches and invalid identifier issues in the Python bindings that could cause import or runtime errors. Fixes #297

addressed issues

  • Convert PyLong_AsLongLong to PyLong_AsLong to match value_holder type
  • Replace invalid argument name with space (layer idx to layer_idx)
  • Add missing pybind11/stl.h include for std::vector return types

changes

  • src/mapnik_value_converter.hpp: Use correct integer conversion
  • src/mapnik_map.cpp: Fix py::arg identifier with invalid space character
  • src/mapnik_font_engine.cpp: Include STL bindings for vector type

These changes fix type mismatches and invalid identifier issues in the Python bindings that could cause import or runtime errors.
- Convert PyLong_AsLongLong to PyLong_AsLong to match value_holder type
- Replace invalid argument name with space (layer idx -> layer_idx)
- Add missing pybind11/stl.h include for std::vector return types

Changes:
- src/mapnik_value_converter.hpp: Use correct integer conversion
- src/mapnik_map.cpp: Fix py::arg identifier with invalid space character
- src/mapnik_font_engine.cpp: Include STL bindings for vector types
Copy link
Member

@artemp artemp left a comment

Choose a reason for hiding this comment

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

I'm curious about #include <pybind11/stl.h> in src/mapnik_font_engine.cpp

Perhaps, related to toolchain used. Which platform/compiler are you using?

@artemp
Copy link
Member

artemp commented Feb 10, 2026

* Convert `PyLong_AsLongLong` to `PyLong_AsLong` to match `value_holder` type

I added static_cast<mapnik::value_integer> as mapnik::value_integer can be std::int64_t or std::int32_t

ref - https://github.com/mapnik/mapnik/blob/cb1e226cc35eec86bb317816dc7942562f548aaa/include/mapnik/value/types.hpp#L47-L56

NOTE: Better approach would be to add a simple traits object to handle correct type conversions at compile time.

@artemp artemp merged commit 1c25a16 into mapnik:master Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] ImportError: Internal error while parsing type signature (1) [HAS FIX]

2 participants