I think I may have the same issue. I find that the CSV importer fails to import a foreign-currency transaction file to a foreign-currency bank account with the exchange-rate conversion properly respected.

Multiple currency accounting works in general for me, for manual entries; typing a transaction/transfer to Supplies into the foreign bank account registoer for 100 foreign currency units (say), appears in my base-currency Supplies register converted to my base currency.

But imported transactions CSV files in foreign currencies to a bank account denominated in a foreign currency, show the same (ie unconverted) number on both the foreign and base currency sides of the transaction.

I have had a bit of time to look at this issue but need more information to properly fix it.

The core problem is a misinterpretation of amount and value in the importer code.

For a balanced, two-split transaction both splits should have the same value albeit of opposite sign. If the splits have the same account commodity the amounts will also be the same except for the opposite sign.

However if one of the splits is linked to an account with a commodity (or currency) different from the transaction's currency it's amount will be different from it's value (in proportion to the exchange rate).

The current importer mixes this up. It will keep the same amounts and will adjust the value proportionally to the exchange rate. This is obviously wrong.

However to fix this, I need to know exactly what a "deposit" in the example transaction means.

What is the exact transaction this example is modeling ?
Is it 10 "Currency1" in Account is being transferred to 5 "Currency2" in TransferAccount ? What should be the transaction currency? "Currency1" or "Currency2" ? In the absence of an explicit commodity column the importer assumes the transaction commodity is the book's default currency.

Let's make a different example to make this more explicit:
Assume you buy 10 shares of a StockX at the exchange rate of 2 €/share

How do you expect a csv file to import this transaction would be modelled ?

Date;Deposit;Account;Transfer Account;Description;Price;Reconciled
10/10/2010;-20;AssetAccount(EUR);ExpenseAccount(StockX);Buy;0.5;n

Or 

Date;Deposit;Account;Transfer Account;Description;Price;Reconciled
10/10/2010;-10;AssetAccount(EUR);ExpenseAccount(StockX);Buy;2;n

Or

Date;Deposit;Account;Transfer Account;Description;Price;Reconciled
10/10/2010;-20;AssetAccount(EUR);ExpenseAccount(StockX);Buy;2;n

Or 

Date;Deposit;Account;Transfer Account;Description;Price;Reconciled
10/10/2010;-10;AssetAccount(EUR);ExpenseAccount(StockX);Buy;0.5;n

That boils down to does "Deposit" stand for "Total value transferred" or "Amount of shares" ?

For a simple two-split single currency transaction it can mean both as they are the same. For multi-currency transactions we need to decide on this generally or allow the user to do so on our behalf. While writing this, I think we may have to replace the "Deposit" selector with "Value" and "Amount" selectors. And "Withdrawal" with "Value (-)" and "Amount (-)".

(In reply to GTI from comment #0)
> Where: Deposit * Price = Value(currency2) => 10 * 0.5 = 5
> 
> This import should produce an transaction without splits where the value
> transferred to ExpenseAccount(currency2) is 5 but fail and produces errors.

Like the importer this is conflating the terms used internally by gnucash.

I'm ignoreing signs in this discussion to not make it more complicated.

10 is the *amount* in Currency1 and assuming that's the book currency this means the total split *value* is 10 as well. 5 is the *amount* in currency2, which valued in Currency1 is still 10, hence *value* of that second split will also be 10.

I've decided to explicitly write it down here so I can re-read it if I start doubting again in the future...

GnuCash's register logic sets the transaction currency to the currency of the account of the register having focus when the user creates the transaction, and the register uses the term "transfer account" to label the "other" account in basic view.

ISTM since the imports use those terms that the importer should follow that practice, so the transaction currency should be the currency of the account in the Account field and not the book currency. Using the book currency creates the potential for creating two prices if neither account is denominated in that currency.

A deposit in an asset account is a debit, so a negative deposit is a credit. An English speaker would find using "deposit" in a liability account to be weird usage but would I think interpret it as a payment, also a debit.

The original test case supplies a price rather than an amount for the second currency, so I'd expect the two splits to be

Account                          Debit     Credit
Asset Account(currency1)                       10
Expense Account(currency2)          20

Both splits would have a value of 10 currency1. If trading accounts are enabled there should be two trading account splits as well. Ideally the importer would just create the two splits and the Transaction balancing code would take care of the trading accounts, but ISTR that there's a substantial amount of trading-account code in register and dialog-transfer so I worry that it's not the case.

Really ideally there'd be an xaccTransCreateSimple(Account* originating, Account* transfer, gnc_numeric orig_amount, gnc_numeric transfer_amount, GncDate posted_date, char* num, char* description, char* notes) and xaccTransCreateSimpleWithPrice(Acccount* originating, Account* transfer, gnc_numeric orig_amount, gnc_numeric price, GncDate, char*, char*, char*) that register, dialog-transfer, and importers would call instead of doing it themselves. One of these years...

(In reply to Geert Janssens from comment #2)
> I have had a bit of time to look at this issue but need more information to
> properly fix it.
> 

Hello Geert!

Thank you for taking the time to address this issue.

This revives me with the GnuCash community, I confess that I was discouraged and disillusioned with our community and this reduced my participation.

Since this issue is right at the entrance to GnuCash, I came to think that GnuCash did not want to open the door for more users.


Well . . .

We have here a salad of terms and concepts and this generates many doubts. You have terms and concepts from code and I have terms and concepts from my point of view, so to make it easier, and to speak the same language, let's try to make this more flexible. And my concepts have also changed since I wrote this problem.


> However to fix this, I need to know exactly what a "deposit" in the example
> transaction means.
> 
> What is the exact transaction this example is modeling ?
> Is it 10 "Currency1" in Account is being transferred to 5 "Currency2" in
> TransferAccount ? . . . 

Yes.


> What should be the transaction currency? "Currency1" or
> "Currency2" ?

Currency2


> In the absence of an explicit commodity column the importer
> assumes the transaction commodity is the book's default currency.
> 
> Let's make a different example to make this more explicit:
> Assume you buy 10 shares of a StockX at the exchange rate of 2 €/share
> 
> How do you expect a csv file to import this transaction would be modelled ?
> 
> Date;Deposit;Account;Transfer Account;Description;Price;Reconciled
> 10/10/2010;-20;AssetAccount(EUR);ExpenseAccount(StockX);Buy;0.5;n
> 
> Or 
> 
> Date;Deposit;Account;Transfer Account;Description;Price;Reconciled
> 10/10/2010;-10;AssetAccount(EUR);ExpenseAccount(StockX);Buy;2;n
> 
> Or
> 
> Date;Deposit;Account;Transfer Account;Description;Price;Reconciled
> 10/10/2010;-20;AssetAccount(EUR);ExpenseAccount(StockX);Buy;2;n
> 
> Or 
> 
> Date;Deposit;Account;Transfer Account;Description;Price;Reconciled
> 10/10/2010;-10;AssetAccount(EUR);ExpenseAccount(StockX);Buy;0.5;n
> 

I have not been working with stocks on GnuCash so I do not feel qualified to answer this question for sure but I would venture to say that this would be the one I expected:

> Date;Deposit;Account;Transfer Account;Description;Price;Reconciled
> 10/10/2010;-10;AssetAccount(EUR);ExpenseAccount(StockX);Buy;0.5;n


> That boils down to does "Deposit" stand for "Total value transferred" or
> "Amount of shares" ?
> 

As I said, I wouldn't know how to safely answer this for shares. This example of mine here refers only to currency pairs.

I'm almost sure that this https://bugs.gnucash.org/show_bug.cgi?id=782141 is related to this one.

(In reply to John Ralls from comment #4)
> ISTM since the imports use those terms that the importer should follow that
> practice, so the transaction currency should be the currency of the account
> in the Account field and not the book currency. Using the book currency
> creates the potential for creating two prices if neither account is
> denominated in that currency.
> 
A good point though there's more to it.

1. The importer has two main formats: single line (one line = one transaction) and multi-line (one line = one split with extra logic to determine transaction boundaries). Multi-line import format doesn't define a transfer account. Instead each split will be on a single line and have an account only. So we need a different mechanism to determine the transaction currency.

2. The import data can explicitly set a transaction commodity, which may be different from the Account commodity (single line import format).

The csv import format is highly variable (contrary to OFX or QIF) in terms of which fields will be available. The importer aims to be as flexible as possible to allow import of as many variations as possible. Almost no field is mandatory except for Date, Description, Account (or on general Base Account) and one of Deposit or Withdrawal. On the other end of the spectrum is a highly detailed csv import which includes full transaction details up until reconcile status (for inter-gnucash import/exports).

The basics are covered pretty well, only multi-currency/commodity data needs improvement, largely due to my (at the time of implementation) limited understanding of how such transactions are recorded in gnucash.

The discussion on this bug so far has uncovered a few issues:
1. how to determine the transaction commodity
2. how to interpret the numbers found in the csv file: should we treat them as values or as amounts (gnucash internal terms).

For 1. how to determine the transaction commodity, I see this logic so far:

a. if the csv data specifies a transaction commodity, use that.
b. in the absence of a transaction commodity
   1. for single line format (which involves Account and optional Transfer Account) Account commodity will be used as transaction commodity
   2. for multi-line csv format it's harder. There are a few options: for example the first split with a price of 1 could determine the transaction commodity. I have no idea how common a transaction would be where a price would be 1 while the split's commodity would not be the transaction commodity. If that would still be too common the only option is to ask the user. That is, the assistant would need an extra page in which the user can explicitly set a transaction currency for those transactions that may be ambiguous. That's quite a bit of extra work and frankly in the current situation I'd just communicate such a situation as unsupported for the time being. That is if ambiguity can arise, either assume one option or refuse to import. Either is to be communicated to the user.

> A deposit in an asset account is a debit, so a negative deposit is a credit.
> An English speaker would find using "deposit" in a liability account to be
> weird usage but would I think interpret it as a payment, also a debit.
> 
Agreed and this is related to my second issue.

> The original test case supplies a price rather than an amount for the second
> currency, so I'd expect the two splits to be
> 
> Account                          Debit     Credit
> Asset Account(currency1)                       10
> Expense Account(currency2)          20
> 
So here you assume the "Deposit" maps to value. The current implementation of the importer maps it to amount. And then fails to do the total math correctly (that particular bit is the essence of this bug).

And now we get to my point 2. Internally we use 3 numbers: value, amount and price. value and amount are stored, price is calculated. The relation between these tree numbers is:
value = amount * price

And if two numbers are given the third can be calculated. As said the current impporter implementation expects amount (in the "Deposit" or "Withdrawal" field) and price.

The question that pops up here is what fields are typically or possibly provided in a csv file ?

Are there cases where the csv import data has a value and amount ? That would probably be the ideal case for our own gnucash export format, as it's the only way to prevent rounding errors in an inter-gnucash export/import. So IMO yes, we should support that.

Are there csv import files out there that provide value and price or amount and price ? I don't know. I don't have multi-currency bank accounts or stock accounts I receive csv files from. But it may well be someone keeps track of stock in an excel sheet and does so in either format.

Regardless I do think it's not too hard to support all three combinations. BUT in that case just "Deposit" or "Withdrawal" is not the proper terms. Instead we will have to switch to "Value" and "Amount" to replace "Deposit" and "Value (inverted)" and "Amount (inverted)" to replace "Withdrawal".
These terms can cover all use cases, including single-currency situations.

For multi-currency/commodity transactions two of the three fields must be set to be able to import. Which fields to set depends on the csv format that's provided. If three are set, price will be ignored.
For single currency transactions, users can choose either value or amount as they are equal.

As adding 4 fields may be a bit unwieldy, we can also consider just adding "Value" and "Amount" and an additional top-level check-box "Values and Amounts are inverted" that takes care of the negation of numbers.

> Both splits would have a value of 10 currency1. If trading accounts are
> enabled there should be two trading account splits as well. Ideally the
> importer would just create the two splits and the Transaction balancing code
> would take care of the trading accounts, but ISTR that there's a substantial
> amount of trading-account code in register and dialog-transfer so I worry
> that it's not the case.
> 
> Really ideally there'd be an xaccTransCreateSimple(Account* originating,
> Account* transfer, gnc_numeric orig_amount, gnc_numeric transfer_amount,
> GncDate posted_date, char* num, char* description, char* notes) and
> xaccTransCreateSimpleWithPrice(Acccount* originating, Account* transfer,
> gnc_numeric orig_amount, gnc_numeric price, GncDate, char*, char*, char*)
> that register, dialog-transfer, and importers would call instead of doing it
> themselves. One of these years...

Trading accounts behaviour is another bit to verify as we go along. Good to have brought it up.

(In reply to GTI from comment #5)
> (In reply to Geert Janssens from comment #2)
> > I have had a bit of time to look at this issue but need more information to
> > properly fix it.
> > 
> 
> Hello Geert!
> 
> Thank you for taking the time to address this issue.
> 
> This revives me with the GnuCash community, I confess that I was discouraged
> and disillusioned with our community and this reduced my participation.
> 
> Since this issue is right at the entrance to GnuCash, I came to think that
> GnuCash did not want to open the door for more users.
> 
> 
> Well . . .
> 
> We have here a salad of terms and concepts and this generates many doubts.
> You have terms and concepts from code and I have terms and concepts from my
> point of view, so to make it easier, and to speak the same language, let's
> try to make this more flexible. And my concepts have also changed since I
> wrote this problem.
> 
> 
> > However to fix this, I need to know exactly what a "deposit" in the example
> > transaction means.
> > 
> > What is the exact transaction this example is modeling ?
> > Is it 10 "Currency1" in Account is being transferred to 5 "Currency2" in
> > TransferAccount ? . . . 
> 
> Yes.
> 
> 
> > What should be the transaction currency? "Currency1" or
> > "Currency2" ?
> 
> Currency2

This contradicts how gnucash works internally, though it matches what the current csv exporter does. I'm inclined to fix the exporter for a more consistent behaviour. So a future version of the csv importer will likely work on a different assumption. See comment 7 for more background.

(In reply to GTI from comment #6)
> I'm almost sure that this https://bugs.gnucash.org/show_bug.cgi?id=782141 is
> related to this one.

It is in the sense that it's currently impossible to import values and amounts, which is the only guaranteed way to avoid rounding errors.

> So here you assume the "Deposit" maps to value. The current implementation of the 
> importer maps it to amount. And then fails to do the total math correctly (that 
> particular bit is the essence of this bug).

On the contrary, I assume that the number is the amount of currency1 credited to Account. Since that's the transaction currency it's also the value of that split.

In order to balance, the value of the second split must also be 10 currency1, and since we're given a price of .5 I computed the amount in currency2 as 10/.5 = 20.

> And now we get to my point 2. Internally we use 3 numbers: value, amount and price. 
> value and amount are stored, price is calculated. The relation between these tree 
> numbers is:
> value = amount * price

> And if two numbers are given the third can be calculated. As said the current 
> impporter implementation expects amount (in the "Deposit" or "Withdrawal" field) and 
> price.

That leaves unstated that in the single-line case we have only 2 of the 6 numbers so we must postulate that the price on the Account split is 1, that the given price is for the Transfer_account split, and that the transaction balances so the two splits values are equal and in currency1.

In order to address the round-the-right-number problem it should be possible for a single-line CSV to contain both the Account amount and the Transfer_account amount, just as the Transfer Dialog allows the user to provide either the price or the amount.

Any news ?