Problem with US Tax Report and TXF export
Alex Aycinena
alex.aycinena at gmail.com
Wed Feb 5 20:02:16 EST 2025
John,
Thanks for your quick response.
If I followed you correctly, this is what I will change and test:
gint64
xaccAccountGetTaxUSCopyNumber (const Account *acc)
{
auto copy_number = get_kvp_int64_path (acc, {"tax-US", "copy-number"});
if (copy_number && *copy_number == 0)
/* this is here to clean up any KVPs set to zero due to prior
bug */
/* if copy_number exists and its value is zero get rid of KVP
and return 1 */
{
set_kvp_int64_path (acc, {"tax-US", "copy-number"},
std::nullopt);
return 1;
}
return (copy_number && *copy_number) ? *copy_number : 1;
}
void
xaccAccountSetTaxUSCopyNumber (Account *acc, gint64 copy_number)
{
set_kvp_int64_path (acc, {"tax-US", "copy-number"}, (copy_number
&& *copy_number != 0) ? copy_number : std::nullopt);
}
In the 'Set' routine, I added the need for the copy_number to exist and the
value to not be zero, otherwise the KVP is removed.
Appreciate your feedback.
Reagrds,
Alex
On Tue, Feb 4, 2025 at 8:55 PM John Ralls <jralls at ceridwen.us> wrote:
> Alex,
>
> That commit is part of https://github.com/Gnucash/gnucash/pull/1965.
>
> Your first proposed change isn’t quite right because a the
> std::optional<T>::operator==(T) returns false if the optional doesn’t
> contain a value (see cases 21-33 in
> https://en.cppreference.com/w/cpp/utility/optional/operator_cmp). You want
> (copy_number && *copy_number) ? *copy_number : 1;
>
> *But* the assumption behind the original code is that if the copy_number
> was 0 then get_kvp_int64_path would have returned std::nullopt because
> set_kvp_path<T>(path, nullptr) calls (indirectly) KvpFrame::;set_impl(
> https://github.com/Gnucash/gnucash/blob/b5c1937702ca7a8fba772a47b2ac7fa6b8857bf5/libgnucash/engine/kvp-frame.cpp#L101)
> and that removes the path if it’s passed a false value; nulltpr is false.
> qof_instance_get_path_kvp
> https://github.com/Gnucash/gnucash/blob/b5c1937702ca7a8fba772a47b2ac7fa6b8857bf5/libgnucash/engine/qofinstance.cpp#L1068)
> bases that on getting a nullptr from KvpFrame::get_slot (
> https://github.com/Gnucash/gnucash/blob/b5c1937702ca7a8fba772a47b2ac7fa6b8857bf5/libgnucash/engine/kvp-frame.cpp#L144
> ).
>
> I think you’re right about
>
> void
> xaccAccountSetTaxUSCopyNumber (Account *acc, gint64 copy_number)
> {
> set_kvp_int64_path (acc, {"tax-US", "copy-number"}, copy_number);
> }
>
>
> The problem is that it’s
> set_kvp_int64_path (Account *acc, const Path& path,
> std::optional<gint64> value)
> So gint64 copy_number is getting implicitly converted to
> std::optional<gint64> and that’s what’s passed to
> qof_instance_set_kvp_path<T> (
> https://github.com/Gnucash/gnucash/blob/b5c1937702ca7a8fba772a47b2ac7fa6b8857bf5/libgnucash/engine/qofinstance.cpp#L1076)
> it’s (quite reasonably) the optional that’s tested, not the optional’s
> contained value. After all, it’s copy_number that’s the special case where
> 0 isn’t a valid value.
>
> The fix (and answer to your question about how to get rid of the KVP) is
>
> void
> xaccAccountSetTaxUSCopyNumber (Account *acc, gint64 copy_number)
> {
> set_kvp_int64_path (acc, {"tax-US", "copy-number"}, copy_number ?
> std::make_optional<gint64>(copy_number) : std::nullopt);
> }
>
> so that it will evaluate false in qof_instance_set_kvp_path.
>
> Regards,
> John Ralls
>
>
> On Feb 4, 2025, at 18:04, Alex Aycinena <alex.aycinena at gmail.com> wrote:
>
> I would request some advice in solving a problem that has been
> discussed on the gnucash-users list recently.
>
> The symptom: a US-based user has reported that on the Accounts tab, when
> the Tax Info column is displayed, some accounts show 'Sched B: Total
> dividend income' for their assigned tax codes and others show 'Sched B(0):
> Total dividend income'. The number in parentheses is the 'copy number'
> which should never be zero and should only show up if it is greater than 1
> (and the copy number issue doesn't even apply to Schedule B).
>
> The problem: investigation shows that not only is the display in the Tax
> Info column on the Accounts tab wrong and confusing, but the US Tax Report
> can show incomplete information and the TXF export also comes out with
> incomplete data and invalid data (there is a copy number field that comes
> out as 'C0', which is not valid).
>
> The cause: a commit was made on Oct 17, 2024, commit 63deaad, that changed
> logic, in Account.cpp:
>
> From:
>
> gint64
> xaccAccountGetTaxUSCopyNumber (const Account *acc)
> {
> gint64 copy_number = 0;
> GValue v = G_VALUE_INIT;
> g_return_val_if_fail(GNC_IS_ACCOUNT(acc), FALSE);
> qof_instance_get_path_kvp (QOF_INSTANCE(acc), &v, {"tax-US",
> "copy-number"});
> if (G_VALUE_HOLDS_INT64 (&v))
> copy_number = g_value_get_int64 (&v);
>
> return (copy_number == 0) ? 1 : copy_number;
> }
>
> to:
>
> gint64
> xaccAccountGetTaxUSCopyNumber (const Account *acc)
> {
> auto copy_number = get_kvp_int64_path (acc, {"tax-US", "copy-number"});
> return copy_number ? *copy_number : 1;
> }
>
> If I read it correctly, this changed the logic from "if there is a copy
> number of '0', return '1'", to "if there is a copy number (even if it is
> zero) just return it, otherwise return '1'".
>
> Also changed in commit 63deaad,
>
> From:
>
> void
> xaccAccountSetTaxUSCopyNumber (Account *acc, gint64 copy_number)
> {
> g_return_if_fail(GNC_IS_ACCOUNT(acc));
> xaccAccountBeginEdit (acc);
> if (copy_number != 0)
> {
> GValue v = G_VALUE_INIT;
> g_value_init (&v, G_TYPE_INT64);
> g_value_set_int64 (&v, copy_number);
> qof_instance_set_path_kvp (QOF_INSTANCE (acc), &v, {"tax-US",
> "copy-number"});
> }
> else
> {
> qof_instance_set_path_kvp (QOF_INSTANCE (acc), nullptr, {"tax-US",
> "copy-number"});
> }
> mark_account (acc);
> xaccAccountCommitEdit (acc);
> }
>
> to
>
> void
> xaccAccountSetTaxUSCopyNumber (Account *acc, gint64 copy_number)
> {
> set_kvp_int64_path (acc, {"tax-US", "copy-number"}, copy_number);
> }
>
> Again, if I read it correctly, this changed the logic from "if the copy
> number is not zero, create a new KVP for it and save it or, if there is
> already a KVP for 'copy number', change it to the new value, otherwise,
> delete the KVP (but do not save a zero value)" to "just save the value,
> even if it is zero".
>
> This change caused the problem and symptoms.
>
> The fix: first correct/retore the logic in '
> xaccAccountGetTaxUSCopyNumber' and ' xaccAccountGetTaxUSCopyNumber'.
> Second, if any of our users have used 'Edit->Tax Report Options' to modify
> their tax code assignments for releases since Oct 17, 2024, they may have
> data files with 'copy number' KVPs with zeros and inaccurate US Income Tax
> Reports/TXF Exports. We need to figure out how to clean up their data files.
>
> The first correction (I implemented this first in 2009, so not sure of
> current syntax, thus need your feedback):
>
> for ' xaccAccountGetTaxUSCopyNumber'
>
> - return copy_number ? *copy_number : 1;
> + return (copy_number == 0) ? 1 : copy_number;
>
>
> for 'xaccAccountSetTaxUSCopyNumber'
>
> - set_kvp_int64_path (acc, {"tax-US", "copy-number"}, copy_number);
> + if (copy_number != 0)
> + {
> + set_kvp_int64_path (acc, {"tax-US", "copy-number"},
> copy_number);
> + }
> + else
> + {
> + Get rid of KVP {"tax-US", "copy-number"}); <- how do I do this
> now!
> + }
>
> For the second correction (clean up users data files), I was thinking to
> add logic to 'xaccAccountGetTaxUSCopyNumber' that would, if it gets a zero
> value, delete the KVP, so it would look like this:
>
> gint64
> xaccAccountGetTaxUSCopyNumber (const Account *acc)
> {
> auto copy_number = get_kvp_int64_path (acc, {"tax-US", "copy-number"});
> if (copy_number == 0)
> {
> Get rid of KVP {"tax-US", "copy-number"}); <- how do I do
> this now!
> return 1;
> }
> return copy_number;
> }
>
> Since 'xaccAccountGetTaxUSCopyNumber' is basically used in only three
> places ('Edit->Tax Report Options', the 'Tax Info' column of the Accounts
> tab, and the 'US Tax Report"), the problem would simply be cleaned up by
> being called through normal activity (looking at the data or running a
> report).
>
> Feedback welcome,
>
> Alex
> _______________________________________________
> gnucash-devel mailing list
> gnucash-devel at gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20250205/8ffa9fa1/attachment.htm>
More information about the gnucash-devel
mailing list