Skip to content

Conversation

@pitkant
Copy link
Member

@pitkant pitkant commented Mar 31, 2025

Changes made to CITATION in a nutshell:

  • Added DOI from CRAN (possible conflict with DOI from Zenodo?)
  • Updated CITATION so that it updates dates and version numbers automatically, also has a bibentry-key now as well
  • Removed old textVersion to rely on the automatically formed APA-style citation

Changes made to pxweb_get in a nutshell:

  • Hack in support for handling json-stat2 files by pretending that they are like json-stat (might be a problem but it works)
  • Add example query downloaded from StatFin website that has a slightly different structure to the examples in the package. Add one line of code that removes the unnecessary parts of this json file to make it similar to previous examples

Changes made to tests in a nutshell:

  • Changes in expected outputs to match current situation
  • Fixed cases where old 10-year binning was used, which caused failures
  • Made the big test slightly smaller
  • Added on 10 second wait in test-pxweb_get.R to avoid hitting API limits (although running the tests takes quite a lot of time already...)

I noticed, while intending to cite pxweb in a publication, that the citation, or more specifically the citation("pxweb") for pxweb CRAN release 0.17.0 was inadvertently out of date. I changed the CITATION file so that it now automatically updates the year and version number of the package in the citation. I also removed the textVersion field since it is easily forgotten and becomes out-of-date as well. Without the textVersion field the citation() function outputs a normal text citation

I have also noticed some time ago that json-query files, or at least those downloaded from StatFin website, have an additional queryObj -layer that however is easily removed, making it compatible with the type of json query that can be copy-pasted from the website. I figured it's easy enough a fix to warrant its inclusion in the package

Fixes to tests that were failing, see #279

@pitkant pitkant requested a review from MansMeg March 31, 2025 17:40
@MansMeg
Copy link
Contributor

MansMeg commented Mar 31, 2025

Hi! This sounds like great fixes and additions. However, the tests are failing. Please check the unit tests what might be wrong.

@pitkant
Copy link
Member Author

pitkant commented Mar 31, 2025

@MansMeg I already recognized the failing tests, see issue #279

@codecov
Copy link

codecov bot commented Apr 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.72%. Comparing base (f7da003) to head (f9d9eca).
Report is 16 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #280      +/-   ##
==========================================
- Coverage   89.80%   89.72%   -0.09%     
==========================================
  Files          32       32              
  Lines        1825     1829       +4     
==========================================
+ Hits         1639     1641       +2     
- Misses        186      188       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pitkant pitkant changed the title Dynamic citations, support for JSON-stat2 Dynamic citations, support for JSON-stat2, fix tests Apr 1, 2025
@pitkant
Copy link
Member Author

pitkant commented Apr 1, 2025

@MansMeg tests are now fixed, everything should be ok. I also bumped the patch version up by a notch

@pitkant
Copy link
Member Author

pitkant commented Apr 1, 2025

Test remaining failures seem to be of the type "Error in curl::curl_fetch_memory(url, handle = handle): Failure when receiving data from the peer [api.scb.se]: Recv failure: Connection reset by peer", which implies to me that the test server is sending too many requests to SCB API in too short of a time frame. This could be fixed by adding enough Sys.sleep(10) between the requests but I'm not sure if this is needed since API tests aren't run on CRAN anyway.

Copy link
Contributor

@MansMeg MansMeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

@MansMeg MansMeg merged commit 816e244 into master Apr 1, 2025
12 of 15 checks passed
@pitkant pitkant deleted the pitkant-patch-citation branch December 4, 2025 09:19
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.

3 participants