Feature/map output #160
Feature/map output #160
Conversation
erik-whiting
force-pushed
the
feature/map-output
branch
from
210a24e to
0465f23
Compare
|
looks good from my side. Maybe wait for Julianus to give some feedback. |
There was a problem hiding this comment.
Yes looks good. Also the small refactor is nice.
But did you think about a bigger refactor in a way that there is one big code template for the while loop and the access and conversion of keys and values is just substitutable?
Then you just have very small if-cases in the beginning that setup these variables and way less duplication and complexity.
autowrap/ConversionProvider.py
Outdated
| |cdef libcpp_map[$cy_tt_key, $cy_tt_value].iterator $it = $input_cpp_var.begin() | ||
| |cdef $py_tt_key $item_key | ||
| |while $it != $input_cpp_var.end(): | ||
| | #$output_py_var[$key_conv] = $value_conv |
autowrap/ConversionProvider.py
Outdated
| | #$output_py_var[$key_conv] = $value_conv | ||
| | $item_key = $py_tt_key.__new__($py_tt_key) | ||
| | $item_key.inst = shared_ptr[$cy_tt_key](new $cy_tt_key((deref($it)).first)) | ||
| | # $output_py_var[$key_conv] = $value_conv |
autowrap/ConversionProvider.py
Outdated
| ): | ||
| value_conv = "<%s>(deref(%s).second)" % (cy_tt_value, it) | ||
| key_conv = "deref(<%s *> (<%s> key).inst.get())" % (cy_tt_key, py_tt_key) | ||
| cy_tt = tt_value.base_type |
There was a problem hiding this comment.
cy_tt_value_base? Feel free to use better variable names. You do not need to stay with the old ones.
erik-whiting
force-pushed
the
feature/map-output
branch
from
20bb65d to
5a921ef
Compare
|
@jpfeuffer it's ready for another look whenever you have time |
erik-whiting
force-pushed
the
feature/map-output
branch
2 times, most recently
from
cabf5b6 to
786db2a
Compare
erik-whiting
force-pushed
the
feature/map-output
branch
from
786db2a to
e816e72
Compare
|
Hey @jpfeuffer, I implemented the approach you suggested for building the cython code for Let me know if you want me to change anything. If you merge this, I think I'll use it as the base for future refactoring in that same file, should make it a lot smaller and easier to understand. Lmk! |
|
Yes, a bit better. Still not quite completely refactored IMO: This applies to other if-cases and maybe more variables in the template code. |
This is in support of issue #143, let me know if this is what you're looking for. Thanks!
PS, there are some refactors in this PR