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
_______________________________________________
gnucash-devel mailing list
[email protected]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
