# Wrap meas_astrom with pybind11

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
6
• Sprint:
• Team:

#### Activity

Hide
Russell Owen added a comment -

I had to wrap LinearTransform.operator() in afw.
Otherwise it was straightforward.

However, I am not very happy with astrometry_net.cc. It has a lot of C++ code mixed in with the usual pybind11 wrapper. The extra code is there as a shim to astrometry_net's C code, so it's reasonable to have there, but the file feels cluttered to me. Suggestions for cleanup welcome.

Note that we will be moving our wrapper of astrometry_net into a new package at some point (soon after we switch to pybind11, I hope, but so far other things have always taken priority). So if a very invasive sort of cleanup is wanted, it might be best to defer that until the split.

Show
Russell Owen added a comment - I had to wrap LinearTransform.operator() in afw. Otherwise it was straightforward. However, I am not very happy with astrometry_net.cc . It has a lot of C++ code mixed in with the usual pybind11 wrapper. The extra code is there as a shim to astrometry_net's C code, so it's reasonable to have there, but the file feels cluttered to me. Suggestions for cleanup welcome. Note that we will be moving our wrapper of astrometry_net into a new package at some point (soon after we switch to pybind11, I hope, but so far other things have always taken priority). So if a very invasive sort of cleanup is wanted, it might be best to defer that until the split.
Hide
Pim Schellart [X] (Inactive) added a comment -

Looks good.

I would however prefer to split the C++ shim from the wrapper code.
Feel free to disagree, or choose between a single astronomy_net.h + .cc combo (in the standard include / src locations) or having separate MultiIndex.h + Solver.h (+ .cc) files.

Show
Pim Schellart [X] (Inactive) added a comment - Looks good. I would however prefer to split the C++ shim from the wrapper code. Feel free to disagree, or choose between a single astronomy_net.h + .cc combo (in the standard include / src locations) or having separate MultiIndex.h + Solver.h (+ .cc) files.
Hide
Russell Owen added a comment - - edited

I split the file as you suggested and added MultiIndex.__iter__ in Python. Note that I left the logging functions in the pybind11 wrapper because I think they would need more work to be made visible to C++ code (more than I think is justified at this time).

Show
Russell Owen added a comment - - edited I split the file as you suggested and added MultiIndex.__iter__ in Python. Note that I left the logging functions in the pybind11 wrapper because I think they would need more work to be made visible to C++ code (more than I think is justified at this time). Please have another look.
Hide
Pim Schellart [X] (Inactive) added a comment -

Looks good.

Show
Pim Schellart [X] (Inactive) added a comment - Looks good.

#### People

Assignee:
Russell Owen
Reporter:
Pim Schellart [X] (Inactive)
Reviewers:
Pim Schellart [X] (Inactive)
Watchers:
Krzysztof Findeisen, Pim Schellart [X] (Inactive), Russell Owen