Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commodities only forward pairs are shown #139

Closed
corani opened this issue Feb 18, 2016 · 7 comments
Closed

Commodities only forward pairs are shown #139

corani opened this issue Feb 18, 2016 · 7 comments
Assignees

Comments

@corani
Copy link
Contributor

corani commented Feb 18, 2016

I only have currencies as commodities, and found that the inverse rates weren't shown under commodities due to:

https://github.com/aumayr/fava/blob/master/fava/api/__init__.py#L478

While this would make sense for stocks, for currencies you usually want to see the inverse as well.

@corani
Copy link
Contributor Author

corani commented Feb 18, 2016

As a solution, I have a meta-tag asset-class: "Cash" on all my currency commodities, maybe this could be used to determine for which commodities the inverse should be shown?

@yagebu
Copy link
Member

yagebu commented Mar 13, 2016

If we add something to distinguish between currencies and other commodities, it would be nice if it worked automatically without having to add any metadata. I don't put any commodity directives for currencies anyway.

How about using the following heuristic: If the commodity name has three letters or less (as all the ISO 4217 currency codes do), treat it like a curreny.

@corani
Copy link
Contributor Author

corani commented Mar 14, 2016

It's not ideal, since there certainly are three or less letter stocks, but it would work for me.

Some other ideas:

  • only do this for operating_currency
  • use a (hardcoded) list of currency codes
  • configuration option

@aumayr
Copy link
Member

aumayr commented Mar 14, 2016

How about using the following heuristic: If the commodity name has three letters or less (as all the ISO 4217 currency codes do), treat it like a currency.

This calls for a default of "disabled" and a configuration option to enable it, because it would confuse users if they do not know about this convention. So my vote is for "configuration over convention" ;-)

@yagebu
Copy link
Member

yagebu commented Mar 14, 2016

I guess the heuristic I suggested is actually not a good idea. If possible, I would refrain from adding another option and the following seems to be the best suggestion:

  • only do this for operating_currency

@yagebu yagebu self-assigned this Mar 15, 2016
@yagebu yagebu removed the discussion label Mar 15, 2016
@yagebu yagebu closed this as completed in 016db2b Mar 15, 2016
@aumayr
Copy link
Member

aumayr commented Mar 15, 2016

@yagebu Is the formatting of the numbers in the following screenshot accidental? (CHF/EUR is the calculated one)

screenshot

yagebu added a commit that referenced this issue Mar 15, 2016
@yagebu
Copy link
Member

yagebu commented Mar 15, 2016

@aumayr: turns out that format_currency wasn't used. It is now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants