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

Update gnc-commodity.cpp for finance-quote 1.63 #2033

Open
wants to merge 2 commits into
base: stable
Choose a base branch
from

Conversation

pkryger
Copy link

@pkryger pkryger commented Oct 17, 2024

A few weeks ago @bpschuck asked for help with updating Gnucash for recent version of finance-quote. I hope this work a reasonable starting point.

I run the following code in [finance-quote-wrapper](https://github.com/Gnucash/gnucash/blob/05aadd0/libgnucash/quotes/finance-quote-wrapper.in#L114) to get multiple quote sources (i.e. more than one module):

use Finance::Quote;
my %qf = $q->get_features();
my $qm = $qf{quote_methods};
while (my ($k, $v) = each %$qm) {
    if (@$v > 1) {
        print "$k => @$v\n";
    }
}

Which yielded the following methods:

nasdaq => AlphaVantage FinanceAPI Fool GoogleWeb IEXCloud MarketWatch StockData YahooJSON
india => BSEIndia NSEIndia
europe => ASEGR Bourso BVB Consorsbank Sinvestor Stooq Tradegate XETRA
ukfunds => FTfunds MorningstarUK
canada => AlphaVantage TMX
usa => AlphaVantage FinanceAPI Fool IEXCloud YahooJSON
nyse => AlphaVantage FinanceAPI Fool GoogleWeb IEXCloud MarketWatch StockData YahooJSON

Then got single quote sources list using the script above, but used @$v == 1. Cross checked with finance-quote/Modules-README.yml, and tried to apply the following rules, while updating the list of modules in gnc-commodity.cpp:

  • if method names a country and has a working module - move it to multi source modules,
  • if module has been marked as failing - removed from the list,
  • if module has been marked as working and has been present in the list - left it as is (no further check),
  • if module has been marked as working, was not present in the list, and doesn't need an API key - added it to the list if and only if I could fetch a quote,
  • if module has been marked as working and requires an API key, then added it to the list.

While I think this work is probably a reasonable first cut, I think the following comments and questions should be at least reviewed by someone with more experience in this subject:

  • Used FinanceAPI instead of yahoo_json in multiple quote sources descriptions.
  • India Mutual - intiamutual looks like a multiple quote source, but has only one source, leave as is?
  • Highlighting other single source countries already in list: australia, dutch indiamutual, france, and romania.
  • Added more single source countries to multiple quote sources: india, hungary, greece, and poland.
  • Should aufund be added to multiple quote sources?
  • troweprice has been in multiple quote sources, but it has only one module. Not sure why. Removed from multiple quote sources. Also removed troweprice_direct in favour of troweprice in the single quote sources. Added as alternative in manual.
  • Added both tratedate and xetra as they seem to use more specific URL than sinvestor does.
  • Should tsx be renamed to tmx for Toronto Stock eXchange? The list originally had tsx (left it as is). Alternatively change the name to Toronto Stock eXchange (TMX), CA to highlight what TMX is.
  • bats - omitted in favour of googleweb. Or shall I add it, as say BATS Global Markets, US with a link in manual pointing to https://cboe.com (per Wikipedia)
  • tradeville - omitted in favour of bvb.
  • mstaruk - omitted, in favour of morningstaruk that has already been in the list. Added as alternative in manual.
  • Omitted all sources from hu module, that is failing ATM, that is: bamosz, bet, hufund, hustock, and hu. Moved section to deprecated in manual.
  • morningstarau - omitted, module is failing ATM.
  • oslobors - omitted, module is failing ATM.
  • cse - added, module is actually working (marked as failing in Modules-README.yml, will send a separate PR to address this).
  • comdirect - strictly speaking it is failing in 1.63, but fixed recently - included.

There's a complimentary Gnucash/gnucash-docs#345 to update relevant tables in gnucash-docs manual.

Since the change itself doesn't affect any logic (merely updates list contents) I believe a verification I performed with ninja check in debian:stable container should be sufficient.

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

Successfully merging this pull request may close these issues.

1 participant