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.

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

python_optional not working for float and double types #1367

Closed
springmeyer opened this issue Aug 1, 2012 · 8 comments
Closed

python_optional not working for float and double types #1367

springmeyer opened this issue Aug 1, 2012 · 8 comments
Assignees
Labels
Milestone

Comments

@springmeyer
Copy link
Member

This is totally mysterious. python_optional works for custom classes, but not built in types like double and float.

Hence the failing test:

======================================================================
ERROR: python_tests.object_test.test_markersymbolizer_init
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Library/Python/2.7/site-packages/nose-1.1.2-py2.7.egg/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/Users/dane/projects/mapnik6/tests/python_tests/object_test.py", line 170, in test_markersymbolizer_init
    eq_(p.opacity, 0.5)
TypeError: No to_python (by-value) converter found for C++ type: float
@springmeyer
Copy link
Member Author

worked around in 60d843a

@springmeyer
Copy link
Member Author

re-opening, looks like this code may handle this better: http://toast.sourceforge.net/optional_8hpp-source.html

@springmeyer springmeyer reopened this Dec 12, 2012
@ghost ghost assigned artemp May 9, 2013
@artemp
Copy link
Member

artemp commented May 16, 2013

37cc3de boost::optional to/from Python converter

@artemp artemp closed this as completed May 16, 2013
@springmeyer
Copy link
Member Author

@artemp did you test reverting the workaround at 60d843a ?

@springmeyer
Copy link
Member Author

reopening, ideally we also should fix enough to remove this other workaround: 0972662#bindings/python/mapnik_raster_symbolizer.cpp

@springmeyer springmeyer reopened this May 16, 2013
@springmeyer
Copy link
Member Author

Currently with that workaround removed:

diff --git a/bindings/python/mapnik_raster_symbolizer.cpp b/bindings/python/mapnik_raster_symbolizer.cpp
index 54f66a5..fd4a77c 100644
--- a/bindings/python/mapnik_raster_symbolizer.cpp
+++ b/bindings/python/mapnik_raster_symbolizer.cpp
@@ -27,19 +27,8 @@
 #include <mapnik/raster_symbolizer.hpp>
 #include <mapnik/raster_colorizer.hpp>
 #include <mapnik/image_scaling.hpp>
+#include "python_optional.hpp"

-namespace {
-
-// https://github.com/mapnik/mapnik/issues/1367
-PyObject* get_premultiplied_impl(mapnik::raster_symbolizer & sym)
-{
-    boost::optional<bool> premultiplied = sym.premultiplied();
-    if (premultiplied)
-        return ::PyBool_FromLong(*premultiplied);
-    Py_RETURN_NONE;
-}
-
-}
 using mapnik::raster_symbolizer;

 void export_raster_symbolizer()
@@ -132,7 +121,7 @@ void export_raster_symbolizer()
                       ">>> r.mesh_size = 32\n"
             )
         .add_property("premultiplied",
-                      &get_premultiplied_impl,
+                      &raster_symbolizer::premultiplied,
                       &raster_symbolizer::set_premultiplied,
                       "Get/Set premultiplied status of the source image.\n"
                       "Can be used to override what the source data reports (when in error)\n"

I get:

======================================================================
ERROR: python_tests.object_test.test_raster_symbolizer
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Library/Python/2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/Users/dane/projects/mapnik6/tests/python_tests/object_test.py", line 27, in test_raster_symbolizer
    eq_(s.premultiplied,True)
TypeError: No to_python (by-value) converter found for C++ type: bool

@artemp
Copy link
Member

artemp commented May 16, 2013

@springmeyer OK I'll add more converters tomorrow

@artemp
Copy link
Member

artemp commented May 17, 2013

closing via 392561c

@artemp artemp closed this as completed May 17, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants