Sorry about that previous subject line. :-P

Hi All,

"PR1" in the below plan is addressed with:
https://github.com/Gnucash/gnucash/pull/2186

And I decided to do both "PR2" and "PR3" as:
https://github.com/Gnucash/gnucash/pull/2187

I look forward to your thoughts.

Cheers,

> Noah

> On Feb 9, 2026, at 11:22 AM, [email protected] wrote:

> John,
>> Thanks for the guidance on SWIG typemaps -- while new to me, after
>> some learning, I think that's the right mechanism and I appreciate you
>> pointing me in that direction. The Python bindings barely use input
>> typemaps today, so there's a real opportunity to improve the
>> infrastructure here.
>> After studying the existing typemap patterns in the codebase (the
>> GncOwner input/output typemaps in gnucash_core.i, the time64
>> multi-type acceptance in time64.i, and the GList output typemap in
>> base-typemaps.i), I believe we can do this with no breaking changes
>> and no need for deprecation. The approach is a %typemap(in) for each
>> pointer type that tries the normal SWIG pointer conversion first, and
>> only if that fails, checks for a .instance attribute (which is how the
>> Python wrapper classes store their underlying SWIG pointer). The
>> normal code path has zero overhead -- the fallback only triggers when
>> someone passes a wrapper object to a gnucash_core_c function (and we
>> could introduce a deprecation warning in this pathway).
>> I'm estimating three PRs:
>> PR 1 -- SWIG typemap compatibility layer. A %define macro
>> (GNC_ACCEPT_WRAPPER) in gnucash_core.i that generates input typemaps
>> for each core pointer type (GNCPrice *, gnc_commodity *, Account *,
>> Split *, etc.). This is pure infrastructure -- no behavior changes, no
>> risk. Includes tests verifying that wrapper objects are accepted by
>> gnucash_core_c functions.
>> PR 2 -- Fix the return-type wrapping. Add the missing
>> methods_return_instance dict entries for GncPrice, GncPriceDB,
>> GncCommodity, Account, and GncLot. With the typemaps from PR 1 in
>> place, both old-style (gnucash_core_c direct calls) and new-style
>> (wrapper methods) code works. Includes tests for each newly-wrapped
>> method.
>> PR 3 -- Clean up example scripts. Remove the type(x).__name__ ==
>> 'SwigPyObject' workarounds from the shipped examples
>> (gnc_convenience.py, gncinvoicefkt.py, str_methods.py).
>> PRs 1 and 2 could both target 5.15 (except I have no idea the timeline
>> for that). I'll start with PR 1.
>> 5.16 or later: Optionally remove the fallback to pointer, or just
>> leave it forever since it costs nothing
>> Cheers,
>> Noah
>
>
>
>
>
>
>
>> ----------------------------------------------------------------------
>>
>> Message: 1
>> Date: Sat, 7 Feb 2026 13:56:12 -0800
>> From: [email protected]
>> To: [email protected]
>> Subject: Python bindings -- many methods return raw SWIG pointers
>>         instead of wrapped Python objects
>> Message-ID: <[email protected]>
>> Content-Type: text/plain; charset="UTF-8"
>>
>> Hi All,
>>    I've been a happy GnuCash user for 13 years and am recently finding
>> more time and interest to take on some more advanced use cases and
>> also contribute to the GnuCash project. (and more bandwidth thanks to
>> AI assisted coding)
>>
>> My projects all involve use of the official API python bindings.  So
>> you'll see some bug reports, PR, and other chatter from me about that.
>>
>> Here's a matter that's within my skillet to fix, but the best approach
>> is debatable given one route involves breaking changes to the existing
>> python bindings.  So I wanted to seek other's opinions.
>>
>> Background
>> -----------------------------
>>
>> The Python bindings use a two-layer architecture: SWIG auto-generates
>> a low-level C API (gnucash_core_c), and gnucash_core.py wraps selected
>> methods so they return proper Python objects (GncPrice, GncCommodity,
>> etc.) instead of raw SWIG pointers. This wrapping is done via
>> methods_return_instance() dicts that map method names to their return
>> types.
>>
>> The problem is that these dicts are incomplete. Many methods that
>> return pointers to GnuCash objects are exposed on the Python classes
>> (via add_methods_with_prefix) but never registered for return-type
>> wrapping. They silently return raw SwigPyObject pointers that have no
>> Python methods and can only be used via gnucash_core_c C-level
>> function calls.
>>
>> This is confusing because the methods *appear* to work -- they're
>> callable and return non-None values -- but the return values are
>> unusable through the documented Python API. There's no error, no
>> warning, and no documentation indicating which methods are wrapped and
>> which aren't. The only way to discover the problem is to inspect
>> type() on the return value.
>>
>> I've done a systematic audit of all classes in gnucash_core.py and
>> gnucash_business.py, cross-referencing the C functions exposed by SWIG
>> against the methods_return_instance dicts, and empirically verified
>> each finding. Below are the results.
>>
>>
>> Affected Classes and Methods
>> -----------------------------
>>
>> 1. GncPrice -- no wrapping dict exists at all
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> GncPrice is the worst case. bindings/python/gnucash_core.py line 741
>> calls add_methods_with_prefix('gnc_price_'), which exposes all
>> gnc_price_* functions as methods. But there is no
>> methods_return_instance call for GncPrice anywhere in that file -- not
>> a single method has its return type wrapped.
>>
>>   Method            Currently returns                  Should return
>>   ----------------  --------------------------------   ----------------
>>   get_commodity()   SwigPyObject (raw gnc_commodity *)  GncCommodity
>>   get_currency()    SwigPyObject (raw gnc_commodity *)  GncCommodity
>>   clone(book)       SwigPyObject (raw GNCPrice *)       GncPrice
>>   get_value()       _gnc_numeric (SWIG proxy)           GncNumeric
>>
>> get_value() is a partial case -- the _gnc_numeric SWIG proxy is usable
>> via GncNumeric(instance=val), but this is inconsistent with Split and
>> Transaction where equivalent methods (GetAmount, GetValue, GetBalance,
>> etc.) are all wrapped to return GncNumeric directly.
>>
>> Suggested fix:
>>
>>     GncPrice.add_methods_with_prefix('gnc_price_')
>>
>>     gnc_price_dict = {
>>         'get_commodity': GncCommodity,
>>         'get_currency': GncCommodity,
>>         'clone': GncPrice,
>>         'get_value': GncNumeric,
>>     }
>>     methods_return_instance(GncPrice, gnc_price_dict)
>>
>>
>> 2. GncPriceDB -- PriceDB_dict incomplete
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> The existing PriceDB_dict wraps lookup_latest,
>> lookup_nearest_in_time64, lookup_nearest_before_t64, and some
>> convert_balance methods. But several methods that return the same
>> types are missing.
>>
>> Missing single-value wrappers (should return GncPrice):
>>
>>   Method                                   Currently returns
>> Should return
>>   ---------------------------------------- ------------------------
>> -----------
>>   nth_price(commodity, n)                  SwigPyObject (GNCPrice *)
>> GncPrice
>>   lookup_day_t64(commodity, currency, date) SwigPyObject (GNCPrice *)
>> GncPrice
>>
>> Missing single-value wrapper (should return GncNumeric):
>>
>>   Method                                          Currently returns
>> Should return
>>   ----------------------------------------------- --------------------
>> -----------
>>   convert_balance_nearest_before_price_t64(...)    _gnc_numeric (raw)
>>  GncNumeric
>>
>> Its siblings convert_balance_latest_price and
>> convert_balance_nearest_price_t64 are correctly wrapped.
>>
>> Missing list wrappers (should return list of GncPrice):
>>
>>   Method                                             Currently returns
>>     Should return
>>   --------------------------------------------------
>> --------------------- ----------------
>>   lookup_latest_any_currency(commodity)
>> list[SwigPyObject]    list[GncPrice]
>>   lookup_nearest_before_any_currency_t64(comm, date)
>> list[SwigPyObject]    list[GncPrice]
>>   lookup_nearest_in_time_any_currency_t64(comm, date)
>> list[SwigPyObject]    list[GncPrice]
>>
>> Note: get_latest_price, get_nearest_price, and get_nearest_before_price
>> are NOT bugs -- their C functions return gnc_numeric directly (the
>> price value, not a GNCPrice *), so the raw _gnc_numeric return is the
>> correct C type. They should arguably be wrapped to GncNumeric for
>> consistency, but that's a lower priority.
>>
>> Suggested fix:
>>
>>     PriceDB_dict = {
>>         'lookup_latest': GncPrice,
>>         'lookup_nearest_in_time64': GncPrice,
>>         'lookup_nearest_before_t64': GncPrice,
>>         'nth_price': GncPrice,
>>         'lookup_day_t64': GncPrice,
>>         'convert_balance_latest_price': GncNumeric,
>>         'convert_balance_nearest_price_t64': GncNumeric,
>>         'convert_balance_nearest_before_price_t64': GncNumeric,
>>         'get_latest_price': GncNumeric,
>>         'get_nearest_price': GncNumeric,
>>         'get_nearest_before_price': GncNumeric,
>>     }
>>     methods_return_instance(GncPriceDB, PriceDB_dict)
>>
>>     methods_return_instance_lists(GncPriceDB, {
>>         'get_prices': GncPrice,                              # already
>> done
>>         'lookup_latest_any_currency': GncPrice,              # new
>>         'lookup_nearest_before_any_currency_t64': GncPrice,  # new
>>         'lookup_nearest_in_time_any_currency_t64': GncPrice, # new
>>     })
>>
>>
>> 3. GncCommodity -- two missing wrappers
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> GncCommodity.clone is correctly wrapped (gnucash_core.py line 979),
>> but two other methods that return object pointers are not.
>>
>>   Method              Currently returns                          Should
>> return
>>   ------------------  -----------------------------------------
>> ----------------------
>>   obtain_twin(book)   SwigPyObject (raw gnc_commodity *)
>>  GncCommodity
>>   get_namespace_ds()  SwigPyObject (raw gnc_commodity_namespace *)
>> GncCommodityNamespace
>>
>> Additionally, get_quote_source() and get_default_quote_source() return
>> raw gnc_quote_source * pointers. However, gnc_quote_source has no
>> Python wrapper class, so these are a deeper design gap rather than a
>> simple omission -- there's nothing to wrap *to*. Currently the only
>> way to use them is via
>> gnucash_core_c.gnc_quote_source_get_internal_name(ptr)
>> etc. A proper fix would require creating a new GncQuoteSource wrapper
>> class.
>>
>>
>> 4. Account -- one missing wrapper
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>>   Method                    Currently returns                  Should
>> return
>>   ------------------------  --------------------------------
>>  ------------
>>   get_currency_or_parent()  SwigPyObject (raw gnc_commodity *)
>> GncCommodity
>>
>> The existing account_dict wraps GetCommodity -> GncCommodity but
>> misses get_currency_or_parent, which returns the same C type.
>>
>>
>> 5. GncLot -- two missing wrappers
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>>   Method                  Currently returns      Should return
>>   ----------------------  ---------------------  ---------------
>>   get_balance_before(sp)  raw _gnc_numeric       GncNumeric
>>   get_split_list()        list[SwigPyObject]     list[Split]
>>
>> The existing gnclot_dict wraps get_balance -> GncNumeric but misses
>> get_balance_before. And get_split_list needs
>> method_function_returns_instance_list like Account.GetSplitList and
>> Transaction.GetSplitList already have.
>>
>>
>> The Breaking Change Problem
>> ---------------------------
>>
>> Every fix listed above changes a method's return type from a raw SWIG
>> pointer to a wrapped Python object. These are breaking changes: any
>> existing code that passes these return values to gnucash_core_c
>> C-level functions will break, because those functions expect the raw
>> SWIG pointer, not a wrapper.
>>
>> For example, current workaround code looks like:
>>
>>     from gnucash import gnucash_core_c as gc
>>
>>     raw_price = pricedb.nth_price(commodity, 0)
>>     gc.gnc_price_get_currency(raw_price)      # works today
>>     gc.gnc_price_get_time64(raw_price)
>>
>> After wrapping nth_price -> GncPrice:
>>
>>     price = pricedb.nth_price(commodity, 0)    # now returns GncPrice
>>     gc.gnc_price_get_currency(price)           # BREAKS
>>     price.get_currency()                       # new correct usage
>>
>> The workaround-after-the-fix is to use .instance to extract the raw
>> pointer:
>>
>>     gc.gnc_price_get_currency(price.instance)  # works
>>
>> How many users are affected?
>>
>> Anyone using these methods today has already discovered the raw-pointer
>> problem through trial and error and written gnucash_core_c workarounds.
>> These workarounds are undocumented and fragile. The "break" moves users
>> from an undocumented workaround to the intended API.
>>
>> That said, the Python bindings have been in this state for years, and
>> scripts using the C-level workarounds do exist in the wild (Stack
>> Overflow answers, wiki examples, personal scripts).
>>
>>
>> Possible Approaches
>> -------------------
>>
>> I'd like the developers' input on how to handle this. Some options:
>>
>> Option A: Fix everything, accept the break
>>
>>   Add all missing entries to the methods_return_instance dicts.
>>   Document the change in release notes. This is the cleanest long-term
>>   outcome but breaks existing workaround code silently (no error --
>>   just wrong types passed to C functions, likely causing segfaults or
>>   TypeError).
>>
>> Option B: Fix everything, add a compatibility shim
>>
>>   Modify method_function_returns_instance (in function_class.py) so
>>   that wrapped objects are transparently accepted by gnucash_core_c
>>   functions. This could be done by making the wrapper classes implement
>>   __swig_convert__ or by patching process_list_convert_to_instance to
>>   unwrap at call boundaries. This would make both old and new code
>>   work, but adds complexity to the wrapping layer.
>>
>> Option C: Fix only the most impactful methods, leave the rest
>>
>>   Prioritize the methods most likely to be encountered by users:
>>   - GncPrice.get_commodity(), .get_currency()
>>   - GncPriceDB.nth_price()
>>   - Account.get_currency_or_parent()
>>
>>   Leave edge cases like GncLot.get_balance_before() and
>>   GncCommodity.obtain_twin() for later.
>>
>> Option D: Deprecation warnings first, fix later
>>
>>   Add runtime deprecation warnings when unwrapped methods are called,
>>   pointing users to the upcoming change. Fix the return types in the
>>   next major release.
>>
>> ---
>>
>> My preference is Option A with clear release notes because the
>> compatibility shim may be too complex. The current state is a usability
>> trap -- methods look like they work but return unusable objects -- and
>> fixing it benefits all future users even if it inconveniences the few
>> who have written workarounds.
>>
>> I'm happy to submit patches for whichever approach the project prefers.
>>
>>
>> Appendix: Methodology
>> ---------------------
>>
>> All findings were verified empirically on GnuCash 5.14 built from
>> source (Python 3.11, Debian Bookworm, -DWITH_PYTHON=ON
>> -DWITH_GNUCASH=OFF). Each method was called against a test GnuCash
>> SQLite file containing accounts, commodities, and prices. Return types
>> were checked with type() and compared against the C function signatures
>> in gnc-pricedb.h, gnc-commodity.h, Account.h, and gnc-lot.h.
>>
>> The full C API surface was enumerated via dir(gnucash_core_c) and
>> cross-referenced against the methods_return_instance dicts in
>> gnucash_core.py (lines 769-776, 960-974, 984-992, 1011-1020,
>> 1029-1044, 1056-1074, 1085-1114) and gnucash_business.py.
>>
>>
>> Sincerely,
>>
>> Noah Reddell
>>
>>
>> ------------------------------
>>
>> Message: 2
>> Date: Sat, 7 Feb 2026 15:40:07 -0800
>> From: John Ralls <[email protected]>
>> To: [email protected]
>> Cc: [email protected]
>> Subject: Re: Python bindings -- many methods return raw SWIG pointers
>>         instead of wrapped Python objects
>> Message-ID: <[email protected]>
>> Content-Type: text/plain;       charset=utf-8
>>
>>
>>
>> > On Feb 7, 2026, at 13:56, [email protected] wrote:
>> >
>> > Hi All,
>> >   I've been a happy GnuCash user for 13 years and am recently finding
>> > more time and interest to take on some more advanced use cases and
>> > also contribute to the GnuCash project. (and more bandwidth thanks to
>> > AI assisted coding)
>> >
>> > My projects all involve use of the official API python bindings.  So
>> > you'll see some bug reports, PR, and other chatter from me about that.
>> >
>> > Here's a matter that's within my skillet to fix, but the best approach
>> > is debatable given one route involves breaking changes to the existing
>> > python bindings.  So I wanted to seek other's opinions.
>> >
>> > Background
>> > -----------------------------
>> >
>> > The Python bindings use a two-layer architecture: SWIG auto-generates
>> > a low-level C API (gnucash_core_c), and gnucash_core.py wraps selected
>> > methods so they return proper Python objects (GncPrice, GncCommodity,
>> > etc.) instead of raw SWIG pointers. This wrapping is done via
>> > methods_return_instance() dicts that map method names to their return
>> > types.
>> >
>> > The problem is that these dicts are incomplete. Many methods that
>> > return pointers to GnuCash objects are exposed on the Python classes
>> > (via add_methods_with_prefix) but never registered for return-type
>> > wrapping. They silently return raw SwigPyObject pointers that have no
>> > Python methods and can only be used via gnucash_core_c C-level
>> > function calls.
>> >
>> > This is confusing because the methods *appear* to work -- they're
>> > callable and return non-None values -- but the return values are
>> > unusable through the documented Python API. There's no error, no
>> > warning, and no documentation indicating which methods are wrapped and
>> > which aren't. The only way to discover the problem is to inspect
>> > type() on the return value.
>> >
>> > I've done a systematic audit of all classes in gnucash_core.py and
>> > gnucash_business.py, cross-referencing the C functions exposed by SWIG
>> > against the methods_return_instance dicts, and empirically verified
>> > each finding. Below are the results.
>> >
>> >
>> > Affected Classes and Methods
>> > -----------------------------
>> >
>> > 1. GncPrice -- no wrapping dict exists at all
>> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >
>> > GncPrice is the worst case. bindings/python/gnucash_core.py line 741
>> > calls add_methods_with_prefix('gnc_price_'), which exposes all
>> > gnc_price_* functions as methods. But there is no
>> > methods_return_instance call for GncPrice anywhere in that file -- not
>> > a single method has its return type wrapped.
>> >
>> >  Method            Currently returns                  Should return
>> >  ----------------  --------------------------------   ----------------
>> >  get_commodity()   SwigPyObject (raw gnc_commodity *)  GncCommodity
>> >  get_currency()    SwigPyObject (raw gnc_commodity *)  GncCommodity
>> >  clone(book)       SwigPyObject (raw GNCPrice *)       GncPrice
>> >  get_value()       _gnc_numeric (SWIG proxy)           GncNumeric
>> >
>> > get_value() is a partial case -- the _gnc_numeric SWIG proxy is usable
>> > via GncNumeric(instance=val), but this is inconsistent with Split and
>> > Transaction where equivalent methods (GetAmount, GetValue, GetBalance,
>> > etc.) are all wrapped to return GncNumeric directly.
>> >
>> > Suggested fix:
>> >
>> >    GncPrice.add_methods_with_prefix('gnc_price_')
>> >
>> >    gnc_price_dict = {
>> >        'get_commodity': GncCommodity,
>> >        'get_currency': GncCommodity,
>> >        'clone': GncPrice,
>> >        'get_value': GncNumeric,
>> >    }
>> >    methods_return_instance(GncPrice, gnc_price_dict)
>> >
>> >
>> > 2. GncPriceDB -- PriceDB_dict incomplete
>> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >
>> > The existing PriceDB_dict wraps lookup_latest,
>> > lookup_nearest_in_time64, lookup_nearest_before_t64, and some
>> > convert_balance methods. But several methods that return the same
>> > types are missing.
>> >
>> > Missing single-value wrappers (should return GncPrice):
>> >
>> >  Method                                   Currently returns
>> > Should return
>> >  ---------------------------------------- ------------------------
>> > -----------
>> >  nth_price(commodity, n)                  SwigPyObject (GNCPrice *)
>> GncPrice
>> >  lookup_day_t64(commodity, currency, date) SwigPyObject (GNCPrice *)
>> GncPrice
>> >
>> > Missing single-value wrapper (should return GncNumeric):
>> >
>> >  Method                                          Currently returns
>> > Should return
>> >  ----------------------------------------------- --------------------
>> > -----------
>> >  convert_balance_nearest_before_price_t64(...)    _gnc_numeric (raw)
>> > GncNumeric
>> >
>> > Its siblings convert_balance_latest_price and
>> > convert_balance_nearest_price_t64 are correctly wrapped.
>> >
>> > Missing list wrappers (should return list of GncPrice):
>> >
>> >  Method                                             Currently returns
>> >    Should return
>> >  --------------------------------------------------
>> > --------------------- ----------------
>> >  lookup_latest_any_currency(commodity)
>> > list[SwigPyObject]    list[GncPrice]
>> >  lookup_nearest_before_any_currency_t64(comm, date)
>> > list[SwigPyObject]    list[GncPrice]
>> >  lookup_nearest_in_time_any_currency_t64(comm, date)
>> > list[SwigPyObject]    list[GncPrice]
>> >
>> > Note: get_latest_price, get_nearest_price, and get_nearest_before_price
>> > are NOT bugs -- their C functions return gnc_numeric directly (the
>> > price value, not a GNCPrice *), so the raw _gnc_numeric return is the
>> > correct C type. They should arguably be wrapped to GncNumeric for
>> > consistency, but that's a lower priority.
>> >
>> > Suggested fix:
>> >
>> >    PriceDB_dict = {
>> >        'lookup_latest': GncPrice,
>> >        'lookup_nearest_in_time64': GncPrice,
>> >        'lookup_nearest_before_t64': GncPrice,
>> >        'nth_price': GncPrice,
>> >        'lookup_day_t64': GncPrice,
>> >        'convert_balance_latest_price': GncNumeric,
>> >        'convert_balance_nearest_price_t64': GncNumeric,
>> >        'convert_balance_nearest_before_price_t64': GncNumeric,
>> >        'get_latest_price': GncNumeric,
>> >        'get_nearest_price': GncNumeric,
>> >        'get_nearest_before_price': GncNumeric,
>> >    }
>> >    methods_return_instance(GncPriceDB, PriceDB_dict)
>> >
>> >    methods_return_instance_lists(GncPriceDB, {
>> >        'get_prices': GncPrice,                              # already
>> done
>> >        'lookup_latest_any_currency': GncPrice,              # new
>> >        'lookup_nearest_before_any_currency_t64': GncPrice,  # new
>> >        'lookup_nearest_in_time_any_currency_t64': GncPrice, # new
>> >    })
>> >
>> >
>> > 3. GncCommodity -- two missing wrappers
>> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >
>> > GncCommodity.clone is correctly wrapped (gnucash_core.py line 979),
>> > but two other methods that return object pointers are not.
>> >
>> >  Method              Currently returns                          Should
>> return
>> >  ------------------  -----------------------------------------
>> > ----------------------
>> >  obtain_twin(book)   SwigPyObject (raw gnc_commodity *)
>>  GncCommodity
>> >  get_namespace_ds()  SwigPyObject (raw gnc_commodity_namespace *)
>> > GncCommodityNamespace
>> >
>> > Additionally, get_quote_source() and get_default_quote_source() return
>> > raw gnc_quote_source * pointers. However, gnc_quote_source has no
>> > Python wrapper class, so these are a deeper design gap rather than a
>> > simple omission -- there's nothing to wrap *to*. Currently the only
>> > way to use them is via
>> gnucash_core_c.gnc_quote_source_get_internal_name(ptr)
>> > etc. A proper fix would require creating a new GncQuoteSource wrapper
>> > class.
>> >
>> >
>> > 4. Account -- one missing wrapper
>> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >
>> >  Method                    Currently returns                  Should
>> return
>> >  ------------------------  --------------------------------
>>  ------------
>> >  get_currency_or_parent()  SwigPyObject (raw gnc_commodity *)
>> GncCommodity
>> >
>> > The existing account_dict wraps GetCommodity -> GncCommodity but
>> > misses get_currency_or_parent, which returns the same C type.
>> >
>> >
>> > 5. GncLot -- two missing wrappers
>> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >
>> >  Method                  Currently returns      Should return
>> >  ----------------------  ---------------------  ---------------
>> >  get_balance_before(sp)  raw _gnc_numeric       GncNumeric
>> >  get_split_list()        list[SwigPyObject]     list[Split]
>> >
>> > The existing gnclot_dict wraps get_balance -> GncNumeric but misses
>> > get_balance_before. And get_split_list needs
>> > method_function_returns_instance_list like Account.GetSplitList and
>> > Transaction.GetSplitList already have.
>> >
>> >
>> > The Breaking Change Problem
>> > ---------------------------
>> >
>> > Every fix listed above changes a method's return type from a raw SWIG
>> > pointer to a wrapped Python object. These are breaking changes: any
>> > existing code that passes these return values to gnucash_core_c
>> > C-level functions will break, because those functions expect the raw
>> > SWIG pointer, not a wrapper.
>> >
>> > For example, current workaround code looks like:
>> >
>> >    from gnucash import gnucash_core_c as gc
>> >
>> >    raw_price = pricedb.nth_price(commodity, 0)
>> >    gc.gnc_price_get_currency(raw_price)      # works today
>> >    gc.gnc_price_get_time64(raw_price)
>> >
>> > After wrapping nth_price -> GncPrice:
>> >
>> >    price = pricedb.nth_price(commodity, 0)    # now returns GncPrice
>> >    gc.gnc_price_get_currency(price)           # BREAKS
>> >    price.get_currency()                       # new correct usage
>> >
>> > The workaround-after-the-fix is to use .instance to extract the raw
>> > pointer:
>> >
>> >    gc.gnc_price_get_currency(price.instance)  # works
>> >
>> > How many users are affected?
>> >
>> > Anyone using these methods today has already discovered the raw-pointer
>> > problem through trial and error and written gnucash_core_c workarounds.
>> > These workarounds are undocumented and fragile. The "break" moves users
>> > from an undocumented workaround to the intended API.
>> >
>> > That said, the Python bindings have been in this state for years, and
>> > scripts using the C-level workarounds do exist in the wild (Stack
>> > Overflow answers, wiki examples, personal scripts).
>> >
>> >
>> > Possible Approaches
>> > -------------------
>> >
>> > I'd like the developers' input on how to handle this. Some options:
>> >
>> > Option A: Fix everything, accept the break
>> >
>> >  Add all missing entries to the methods_return_instance dicts.
>> >  Document the change in release notes. This is the cleanest long-term
>> >  outcome but breaks existing workaround code silently (no error --
>> >  just wrong types passed to C functions, likely causing segfaults or
>> >  TypeError).
>> >
>> > Option B: Fix everything, add a compatibility shim
>> >
>> >  Modify method_function_returns_instance (in function_class.py) so
>> >  that wrapped objects are transparently accepted by gnucash_core_c
>> >  functions. This could be done by making the wrapper classes implement
>> >  __swig_convert__ or by patching process_list_convert_to_instance to
>> >  unwrap at call boundaries. This would make both old and new code
>> >  work, but adds complexity to the wrapping layer.
>> >
>> > Option C: Fix only the most impactful methods, leave the rest
>> >
>> >  Prioritize the methods most likely to be encountered by users:
>> >  - GncPrice.get_commodity(), .get_currency()
>> >  - GncPriceDB.nth_price()
>> >  - Account.get_currency_or_parent()
>> >
>> >  Leave edge cases like GncLot.get_balance_before() and
>> >  GncCommodity.obtain_twin() for later.
>> >
>> > Option D: Deprecation warnings first, fix later
>> >
>> >  Add runtime deprecation warnings when unwrapped methods are called,
>> >  pointing users to the upcoming change. Fix the return types in the
>> >  next major release.
>> >
>> > ---
>> >
>> > My preference is Option A with clear release notes because the
>> > compatibility shim may be too complex. The current state is a usability
>> > trap -- methods look like they work but return unusable objects -- and
>> > fixing it benefits all future users even if it inconveniences the few
>> > who have written workarounds.
>> >
>> > I'm happy to submit patches for whichever approach the project prefers.
>> >
>> >
>> > Appendix: Methodology
>> > ---------------------
>> >
>> > All findings were verified empirically on GnuCash 5.14 built from
>> > source (Python 3.11, Debian Bookworm, -DWITH_PYTHON=ON
>> > -DWITH_GNUCASH=OFF). Each method was called against a test GnuCash
>> > SQLite file containing accounts, commodities, and prices. Return types
>> > were checked with type() and compared against the C function signatures
>> > in gnc-pricedb.h, gnc-commodity.h, Account.h, and gnc-lot.h.
>> >
>> > The full C API surface was enumerated via dir(gnucash_core_c) and
>> > cross-referenced against the methods_return_instance dicts in
>> > gnucash_core.py (lines 769-776, 960-974, 984-992, 1011-1020,
>> > 1029-1044, 1056-1074, 1085-1114) and gnucash_business.py.
>> >
>>
>>
>> Thanks for being willing to take this on.
>>
>> Unfortunately we have no idea how many python bindings users there are
>> nor how sophisticated any of them are about working around the bindings?
>> limitations. Our general policy is that we wouldn?t remove API without at
>> least a release or two worth of notice via deprecation warnings, so if we
>> got deprecations in the upcoming 5.15 release we?d wait until the end of
>> the year to remove the API. It?s also not ideal to deprecate API before the
>> replacement is available making your option B the best choice.
>>
>> The breakage problem seems to me to be what SWIG typemaps are for, and it
>> looks to me like the python bindings don?t make much use of typemaps. Did
>> you consider that and if not can you?
>>
>> Regards,
>> John Ralls
>>
>>
>>
>> ------------------------------
>>
>> Subject: Digest Footer
>>
>> _______________________________________________
>> gnucash-devel mailing list
>> [email protected]
>> https://lists.gnucash.org/mailman/listinfo/gnucash-devel
>>
>>
>> ------------------------------
>>
>> End of gnucash-devel Digest, Vol 20, Issue 7
>> ********************************************
>>
>
_______________________________________________
gnucash-devel mailing list
[email protected]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Reply via email to