Details
-
Type:
Improvement
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: afw
-
Labels:
-
Story Points:6
-
Epic Link:
-
Sprint:DRP S17-3, DRP S17-4
-
Team:Data Release Production
Description
The way BaseRecord is wrapped in pybind11 needs some work:
- get and __getitem__ should be wrapped the way they were with SWIG, which is simpler code than was used, and looks identical for the two methods (though that code uses $action so there are subtle differences). Better yet, make the methods identical if possible.
- The wrappings should probably use C++ method _get, which needs to be added in the pybind11 wrapper file, rather than the specializations that use suffixes.
- The C++ wrapper defines _getitem_ for each data type and the code is radically different than for _get_ + suffix. It would be nice if the differences could be eliminated.
Similar issues apply to several other afw::table classes.
Attachments
Issue Links
- blocks
-
DM-9100 Merge pybind11 branches to master
- Done
- is blocked by
-
- Done
-
- Done
- relates to
-
- Done
-
- Done
-
- Invalid
(1 relates to)
Activity
| Description |
The way {{BaseRecord}} is wrapped in pybind11 needs some work:
- {{get}} and {{\_\_getitem\_\_}} should be identical (as they were with SWIG) but they are different and neither one can be used in place of the other. One or two tests fail either way. Furthermore the code bears little resemblance to the original SWIG (for good reasons, but I understand Fred knows how to work around this now). - The wrappings should probably use C++ method {{_get}}, which needs to be added in the pybind11 wrapper file. - The C++ wrapper defines {{_getitem_}} for each data type and the code is radically different than for {{_get_ + suffix}}; I hope it can be eliminated or made the same as {{_get_ + suffix}} and renamed to {{_get}}. |
The way {{BaseRecord}} is wrapped in pybind11 needs some work:
- {{get}} and {{\_\_getitem\_\_}} should be wrapped identically (as they were with SWIG) or should actually be identical if possible. - The wrappings should probably use C++ method {{_get}}, which needs to be added in the pybind11 wrapper file, rather than the specializations that use suffixes. - The C++ wrapper defines {{_getitem_}} for each data type and the code is radically different than for {{_get_ + suffix}}; I hope it can be eliminated or made the same as {{_get_ + suffix}} and renamed to {{_get}}. |
| Description |
The way {{BaseRecord}} is wrapped in pybind11 needs some work:
- {{get}} and {{\_\_getitem\_\_}} should be wrapped identically (as they were with SWIG) or should actually be identical if possible. - The wrappings should probably use C++ method {{_get}}, which needs to be added in the pybind11 wrapper file, rather than the specializations that use suffixes. - The C++ wrapper defines {{_getitem_}} for each data type and the code is radically different than for {{_get_ + suffix}}; I hope it can be eliminated or made the same as {{_get_ + suffix}} and renamed to {{_get}}. |
The way {{BaseRecord}} is wrapped in pybind11 needs some work:
- {{get}} and {{\_\_getitem\_\_}} should be wrapped identically (as they were with SWIG) or should actually be identical if possible. - The wrappings should probably use C++ method {{_get}}, which needs to be added in the pybind11 wrapper file, rather than the specializations that use suffixes. - The C++ wrapper defines {{\_getitem\_}} for each data type and the code is radically different than for {{\_get_ + suffix}}. It would be nice if the differences could be eliminated. |
| Description |
The way {{BaseRecord}} is wrapped in pybind11 needs some work:
- {{get}} and {{\_\_getitem\_\_}} should be wrapped identically (as they were with SWIG) or should actually be identical if possible. - The wrappings should probably use C++ method {{_get}}, which needs to be added in the pybind11 wrapper file, rather than the specializations that use suffixes. - The C++ wrapper defines {{\_getitem\_}} for each data type and the code is radically different than for {{\_get_ + suffix}}. It would be nice if the differences could be eliminated. |
The way {{BaseRecord}} is wrapped in pybind11 needs some work:
- {{get}} and {{\_\_getitem\_\_}} should be wrapped the way they were with SWIG, which is simpler code than was used, and looks identical for the two methods (though that code uses {{$action}} so there are subtle differences). Better yet, make the methods identical if possible. - The wrappings should probably use C++ method {{_get}}, which needs to be added in the pybind11 wrapper file, rather than the specializations that use suffixes. - The C++ wrapper defines {{\_getitem\_}} for each data type and the code is radically different than for {{\_get_ + suffix}}. It would be nice if the differences could be eliminated. |
Fred Moolekamp
made changes -
| Assignee | Fred Moolekamp [ fred3m ] |
| Assignee | Jim Bosch [ jbosch ] |
| Epic Link |
|
| Epic Link |
|
| Epic Link |
|
| Team | Data Release Production [ 10301 ] |
| Status | To Do [ 10001 ] | In Progress [ 3 ] |
| Summary | BaseRecord wrapping of set and get needs work | Use visitor pattern to clean up afw::table pybind11 wrappers |
| Description |
The way {{BaseRecord}} is wrapped in pybind11 needs some work:
- {{get}} and {{\_\_getitem\_\_}} should be wrapped the way they were with SWIG, which is simpler code than was used, and looks identical for the two methods (though that code uses {{$action}} so there are subtle differences). Better yet, make the methods identical if possible. - The wrappings should probably use C++ method {{_get}}, which needs to be added in the pybind11 wrapper file, rather than the specializations that use suffixes. - The C++ wrapper defines {{\_getitem\_}} for each data type and the code is radically different than for {{\_get_ + suffix}}. It would be nice if the differences could be eliminated. |
The way {{BaseRecord}} is wrapped in pybind11 needs some work:
- {{get}} and {{\_\_getitem\_\_}} should be wrapped the way they were with SWIG, which is simpler code than was used, and looks identical for the two methods (though that code uses {{$action}} so there are subtle differences). Better yet, make the methods identical if possible. - The wrappings should probably use C++ method {{_get}}, which needs to be added in the pybind11 wrapper file, rather than the specializations that use suffixes. - The C++ wrapper defines {{\_getitem\_}} for each data type and the code is radically different than for {{\_get_ + suffix}}. It would be nice if the differences could be eliminated. Similar issues apply to several other afw::table classes. |
| Sprint | DRP S17-3 [ 360 ] |
| Story Points | 6 |
| Component/s | afw [ 10714 ] |
| Sprint | DRP S17-3 [ 360 ] | DRP S17-3, DRP S17-4 [ 360, 363 ] |
| Rank | Ranked higher |
| Reviewers | Krzysztof Findeisen, Pim Schellart, Russell Owen [ krzys, pschella, rowen ] | |
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
Krzysztof Findeisen
made changes -
| Reviewers | Krzysztof Findeisen, Pim Schellart, Russell Owen [ krzys, pschella, rowen ] | Pim Schellart, Russell Owen [ pschella, rowen ] |
Pim Schellart [X] (Inactive)
made changes -
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |