Feature/pymetkit #56
Feature/pymetkit #56
Conversation
|
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/c-api #56 +/- ##
=================================================
- Coverage 62.67% 62.64% -0.03%
=================================================
Files 106 106
Lines 6725 6728 +3
Branches 634 634
=================================================
Hits 4215 4215
- Misses 2510 2513 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Note Should not be merged until #57 is merged. |
ChrisspyB
requested review from
tbkr,
simondsmart and
jinmannwong
and removed request for
simondsmart
pymetkit/tests/test_marsrequest.py
Outdated
| assert "class" in requests[0] | ||
| assert requests[1]["levelist"] == "500" | ||
|
|
||
| # @todo: [1] no longer raises an exception. Is this correct? |
pymetkit/tests/test_marsrequest.py
Outdated
| expanded.validate() | ||
| assert req == expanded | ||
|
|
||
| # @todo: [0] and [1] no longer raise an exception. Is this correct? |
|
|
||
| metkit_error_t metkit_paramiterator_delete(const metkit_paramiterator_t* it); | ||
| metkit_iterator_status_t metkit_paramiterator_next(metkit_paramiterator_t* it); | ||
| metkit_iterator_status_t metkit_paramiterator_current(const metkit_paramiterator_t* it, const char** param); |
There was a problem hiding this comment.
If you're going to have a parameter iterator, at least make it able to return the number of values and the values as well.
| metkit_error_t metkit_marsrequest_set_verb(metkit_marsrequest_t* request, const char* verb); | ||
| metkit_error_t metkit_marsrequest_verb(const metkit_marsrequest_t* request, const char** verb); | ||
| metkit_error_t metkit_marsrequest_has_param(const metkit_marsrequest_t* request, const char* param, bool* has); | ||
| metkit_error_t metkit_marsrequest_params(const metkit_marsrequest_t* request, metkit_paramiterator_t** params); |
There was a problem hiding this comment.
I'm not convinced you need a params iterator, as opposed to just returning them by count/index in the same way as you do for the values?
There was a problem hiding this comment.
With the existing C++ implementation, it is not possible to return the params without making a copy (they internally stored in a linked list), unlike the values, so they are less suitable for accessing by index and count
Python interface, based on initial implementation by @jinmannwong.
wip