rOpenGov / pxweb

R tools to access PX-WEB API
http://ropengov.github.io/pxweb
Other
69 stars 31 forks source link

Add new APIs to catalogue #265

Closed pitkant closed 8 months ago

pitkant commented 1 year ago

Added new APIs to api.json. Some databases created with pxweb technology didn't have API enabled or only had test api so they were not added. Some APIs had some problems:

Entities that did not have API available:

National Statistical Service of the Republic of Armenia Nordic Health and Welfare Statistics

Entities with broken APIs:

Örebro kommun (old pxweb API version) EUSTAT (redirects to net-inter-eustat-45.ejgvdns, reported to their helpdesk) INSTAT, Mali (sometimes fails, sometimes produces an error message, sometimes works, unstable internet connection?) Icelandic Centre for Retail Studies (px.rsv.is) (no longer existing, timeouts?)

API works fine but produces a warning:

Sundsvall kommun

See issue #254 for a more detailed list and error messages

Also:

codecov[bot] commented 1 year ago

Codecov Report

Merging #265 (a9381c5) into master (75a4fe4) will decrease coverage by 1.38%. The diff coverage is n/a.

:exclamation: Current head a9381c5 differs from pull request most recent head 8cd37e9. Consider uploading reports for the commit 8cd37e9 to get more accurate results

@@            Coverage Diff             @@
##           master     #265      +/-   ##
==========================================
- Coverage   91.53%   90.16%   -1.38%     
==========================================
  Files          32       32              
  Lines        1749     1749              
==========================================
- Hits         1601     1577      -24     
- Misses        148      172      +24     

see 2 files with indirect coverage changes

pitkant commented 1 year ago

Was issue #250 already implemented so that API limits were unneeded in API catalogue? For now I updated them manually but if they can be safely removed I can do that as well

MansMeg commented 1 year ago

Hreat work pyry. There seem to be some issues on the APIs that fails. Maybe move checking the api catalogue to only tests run on test branch.

pitkant commented 1 year ago

I fixed the error on test coverage

Failure ('test-pxweb_api_catalogue.R:18:3'): pxweb_api_catalogue ────────────
pxac[[2]] not equal to pxacgh[[2]].

by changing pxac[[2]] and pxacgh[[2]] to pxac[[1]] and pxacgh[[1]], effectively making the test compare the first item on API list (SCB).

There was the following NOTE on R-CMD-check:

Package CITATION file contains call(s) to old-style personList() or
  as.personList().  Please use c() on person objects instead.
  Package CITATION file contains call(s) to old-style citEntry().  Please
  use bibentry() instead.

The failed test on R-CMD-CHECK was related to running pxweb.sh from tests_bash folder. There indeed seems to be a problem related to permissions:

Run ./tests_bash/pxweb.sh
/home/runner/work/_temp/d3479b69-53c8-4474-9f18-827ea6e2c4ef.sh: line 1: ./tests_bash/pxweb.sh: Permission denied

What do you mean by "move checking the api catalogue to only tests run on test branch"?

MansMeg commented 1 year ago

Hmm. This is a new thing. The same code has worked before.

Could you try to add chmod? See below. https://aileenrae.co.uk/blog/github-actions-shell-script-permission-denied-error/

pitkant commented 1 year ago

I could do that. But at least when I tried running tests_bash/pxweb.R on my computer it took a very long time to do the

A PXWEB API IS IDENTIFIED:
https://web.dzs.hr/PXWeb/api/v1/en/ 
Exploring nodes...
18 node(s) and 0 table(s). Checking entry 1 of 18 ...
18 node(s) and 2 table(s). Checking entry 2 of 20 ...
20 node(s) and 2 table(s). Checking entry 3 of 22 ...
20 node(s) and 2 table(s). Checking entry 4 of 22 ...
22 node(s) and 2 table(s). Checking entry 5 of 24 ...
23 node(s) and 2 table(s). Checking entry 6 of 25 ...
40 node(s) and 2 table(s). Checking entry 7 of 42 ...
44 node(s) and 2 table(s). Checking entry 8 of 46 ...
52 node(s) and 2 table(s). Checking entry 9 of 54 ...
53 node(s) and 2 table(s). Checking entry 10 of 55 ...
[...]
117 node(s) and 695 table(s). Checking entry 812 of 812 ...

phase, surely running something like that on Github Actions is not a good idea? Actually some APIs were so big that they timed out before

PXWEB API CONTAIN:
117 node(s) and 695 table(s) in total.
Downloading data...
  |=============================================================                                  |  45%
Time limit reached. Aborting.

was finished.

pitkant commented 1 year ago

I did a new PR #267 that updates the R-CMD-check workflows to more modern ones. I did not include rows 90-99 from the current workflow:

- name: Check
        env:
          _R_CHECK_CRAN_INCOMING_REMOTE_: false
        run: rcmdcheck::rcmdcheck(args = c("--no-manual", "--as-cran"), error_on = "warning", check_dir = "check")
        shell: Rscript {0}

      - name: Ping API catalogue
        if: matrix.config.r == 'release' && runner.os == 'Linux'
        run: ./tests_bash/pxweb.sh
        shell: bash

I'm sure the workflow could be configured to check --as-cran as before but I'm not sure if that's necessary at all steps. Additionally, the pxweb.sh section is now completely removed, as I was not totally sure why such a time-consuming script should be run every time updates are made.

MansMeg commented 8 months ago

I added the other PR so I close this one npw. Feel free to reopen iof this was incorrect