search

libdynd / dynd-python

Python exposure of dynd
http://libdynd.org
Other
120 stars 23 forks source link

Remove calls to type constructors removed in libdynd #707

Closed insertinterestingnamehere closed 8 years ago

insertinterestingnamehere commented 8 years ago

Bugfixes to bring dynd-python into sync with https://github.com/libdynd/libdynd/pull/1254. Several bits of random cleanup that I took care of along the way are also included. We should discuss what the best way is to map type ids back to types (or how to eliminate a few remaining cases where that needs to happen). Here I ended up having to use dynd::ndt::infos() in a few places, and that seems less than ideal.

izaid commented 8 years ago

Thanks a lot for this. Did you see the same weird type.h errors I saw?

What's happening here is that the API for a few things are going to change. Since we're going to allow types to have base types (and not just base IDs), some of the information that is currrently in the type registry will become methods in ndt::type. Just currently in transition.

insertinterestingnamehere commented 8 years ago

Right. If I remember right, those particular errors were from an include order issue. When I fixed the code in ndt_type_from_pylist (formerly located in type_conversions.hpp) to not use the old constructor, it triggered a clang-format run that changed the include order so that type_api.h was included first. Cython doesn't currently include other headers in their API headers (arguably either a bug or a not-supported feature), so the error reflects that no DyND header had been included when dynd::ndt::type was referenced in that header. We should have had some whitespace there to prevent the re-ordering. Ironically, ndt_type_from_pylist has no business being in the type_conversions files anyway. I went ahead and moved it into the type_deduction files instead.