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

fix CRAN errors #119

Closed
adibender opened this issue Jan 9, 2020 · 5 comments
Closed

fix CRAN errors #119

adibender opened this issue Jan 9, 2020 · 5 comments

Comments

@adibender
Copy link
Owner

@adibender adibender commented Jan 9, 2020

https://cran.r-project.org/web/checks/check_results_coalitions.html

Dear maintainer,

Please see the problems shown on
https://cran.r-project.org/web/checks/check_results_coalitions.html.

Please correct before 2018-10-08 to safely retain your package on CRAN.

Best,
-k

And again. It seems we need to remind you of the CRAN policy:

'Packages which use Internet resources should fail gracefully with an
informative message if the resource is not available (and not give a
check warning nor error).'

Please submit a fully compliant update ASAP and before Jan 23.

@adibender
Copy link
Owner Author

@adibender adibender commented Jan 9, 2020

@bauer-alex The errors on CRAN are due to #118 that you fixed. But I wonder if there is something else we need to consider?

'Packages which use Internet resources should fail gracefully with an
informative message if the resource is not available (and not give a
check warning nor error).'

@bauer-alex
Copy link
Collaborator

@bauer-alex bauer-alex commented Jan 9, 2020

Ok...
Cannot be really due to my commit though, the change wasn't that severe, and didn't change any web scraping functionality whatsoever.
Did we already have some informative error message (or whatever is needed) up to now for when the resource is not available (or when there is no Internet connection)?

I could take a look at it on Monday at the earliest.

@adibender
Copy link
Owner Author

@adibender adibender commented Jan 10, 2020

No, I meant that the error was because of the changes at wahlrecht.de and it will be fixed as soon as we push your commits. But I think the comment from CRAN goes further than the specific error.

Based on feedback from CompStat people I think we need to:
a) Within the scrapper functions check if connection to wahlrecht (or other specific URLs) can be established and return informative error messages if it does not.
b) Check if the dowloaded tables have the proper dimensions/properties before preprocessing and through informative error otherwise
c) Construct a "switch" or something like that for tests/examples, where only tests that do not depend on internet resources are run on CRAN. The scrapper tests can still be run locally/on github/travis, but not on CRAN.

@bauer-alex
Copy link
Collaborator

@bauer-alex bauer-alex commented Jan 14, 2020

The robustify_scrapers branch contains some changes to resolve the issues:

a) Within the scrapper functions check if connection to wahlrecht (or other specific URLs) can be established and return informative error messages if it does not.

The function try_readHTML (I initially called it try_htmlSource, sorry for the confusion!) gives an error message when the website cannot be resolved. We could also check if an internet connection is active in the first place, but a short research lead me to no neat solution.

b) Check if the dowloaded tables have the proper dimensions/properties before preprocessing and through informative error otherwise

I think that's principally good practice, but I don't think we need this to resolve the current CRAN issues. Accordingly, I did nothing in this regard so far.

c) Construct a "switch" or something like that for tests/examples, where only tests that do not depend on internet resources are run on CRAN. The scrapper tests can still be run locally/on github/travis, but not on CRAN.

The tests that depend on a web connection now contain testthat::skip_on_cran() calls. This causes all subsequent tests in the current test_that() call to be skipped on CRAN.

@adibender Would be great if you could take up the work again from here. I'm quite busy the next days.

@adibender
Copy link
Owner Author

@adibender adibender commented Jan 14, 2020

Perfect, thx! Will do.

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

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.