Complete remaining Python binding object support - #165
adrianweidig wants to merge 2 commits into
Conversation
|
Thanks for the proposed changes. Note that there are many competing priorities on my plate at the moment, will take a closer look as soon as time permits. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #165 +/- ##
==========================================
+ Coverage 29.20% 29.22% +0.02%
==========================================
Files 64 64
Lines 20854 20791 -63
Branches 5037 5037
==========================================
- Hits 6090 6076 -14
+ Misses 12818 12769 -49
Partials 1946 1946 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| return( &pypff_message_type_object ); | ||
|
|
||
| case LIBPFF_ITEM_TYPE_TASK: | ||
| case LIBPFF_ITEM_TYPE_TASK_REQUEST: |
There was a problem hiding this comment.
IPM.Task and IPM.TaskRequest are different object types, but both are messages (IPM)
| "Retrieves the type of the value specified by the index." }, | ||
|
|
||
| { "get_value", | ||
| (PyCFunction) pypff_multi_value_get_value, |
There was a problem hiding this comment.
Given that pypff_multi_value_get_value, get_value_as_binary_data and get_value_as_guid all return binary data. Was your intent here to replicate the C API?
| #if PY_MAJOR_VERSION >= 3 | ||
| if( value_type == LIBPFF_VALUE_TYPE_INTEGER_16BIT_SIGNED ) | ||
| { | ||
| integer_object = PyLong_FromLong( |
There was a problem hiding this comment.
Instead of creating a Python object here (and potentially have to decref it on error), why not store the integer value as a C int and convert to a Python object at the end of the function?
|
|
||
| case LIBPFF_VALUE_TYPE_INTEGER_64BIT_SIGNED: | ||
| case LIBPFF_VALUE_TYPE_FILETIME: | ||
| case LIBPFF_VALUE_TYPE_FLOATINGTIME: |
There was a problem hiding this comment.
Isn't this a floating-point value?
| import pypff | ||
|
|
||
|
|
||
| class ModuleTypeTests(unittest.TestCase): |
There was a problem hiding this comment.
What's your rationale for having this test?
|
Comparable changes have been made for most of the proposed changes (open points in issue 2), and #2 has been updated with additional changes that still need to be made. Closing this PR. |
2 participants
Summary
Complete the remaining Python object-model work tracked in #2:
record_entry.name_to_id_map_entryas a typed Python object with type, numeric/string name, and GUID accessorsrecord_entry.multi_valueas a typed Python object with raw, typed, integer, datetime, string, binary-data, and GUID accessorsmessage_storetype fromfile.message_storetasktype for task and task-request itemsrecipientsplaceholder with recipient count, indexed lookup, and sequence accessContext
#164 was closed as a duplicate because the Python bindings are not considered ready. The maintainer pointed to #2, whose remaining unchecked items are the scope of this change. This PR intentionally does not alter the PyPI publishing workflow; it addresses the stated binding-readiness prerequisite instead.
Verification
python -m compileall -q testsgit diff --check(withcr-at-eolfor the repository's CRLF.vcproj)The PR is a draft until the upstream native/wheel checks have completed.