Skip to content

Disable code chunk evaluation in network vignette#348

Merged
cforgaci merged 23 commits into
mainfrom
347-net-vig-http-fail-fix-cf
Nov 24, 2025
Merged

Disable code chunk evaluation in network vignette#348
cforgaci merged 23 commits into
mainfrom
347-net-vig-http-fail-fix-cf

Conversation

@cforgaci
Copy link
Copy Markdown
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Code chunk evaluation is disabled to avoid errors from failed http requests on CRAN. I also precumputed the plot and added it as an image via markdown.

Related Issues

Added/updated tests?

We encourage you to keep the code coverage percentage at 75% and above.

Added entry in changelog?

  • Yes

@cforgaci cforgaci requested a review from fnattino November 12, 2025 07:52
Copy link
Copy Markdown
Contributor

@fnattino fnattino left a comment

Choose a reason for hiding this comment

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

Looks good, thanks. The more I see these issues, the more I think the solution of pre-computing vignettes, also linked in the chapter of rOpenSci you linked, is the way to go if you have any dependence on remote data..

Concerning testing for "graceful" failures: as far as I understand devtools::release_questions() can be used to only add reminders that pops up when you run devtools::release() - a way that one might use to remind yourself of updating a "frozen" vignette rather than a real check.

I am not sure about whether there is a straightforward way to test for graceful vignettes/tests/example failures. The easiest way would be to try to run something like R CMD check offline - but there seem to be no such option.

Alternatively, one could work on the code and make sure that all functions fail gracefully when resources are not available. This would probably help in the tests (which BTW, are currently safe for rcrisp), but not in the examples and vignettes: one should embed all the logic of skipping subsequent code blocks in the examples/vignettes themselves (and yet, you would not have a mechanism to effectively check on whether the behaviour is correct)..

Comment thread cran-comments.md Outdated
@cforgaci cforgaci requested a review from fnattino November 15, 2025 16:02
Copy link
Copy Markdown
Contributor

@fnattino fnattino left a comment

Choose a reason for hiding this comment

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

Just noticed we have yet another failing check due to tests not being able to access the data - even though I thought we have skipped all tests with dependencies on the example data on CRAN. I am going to try to investigate this a bit further.

https://www.r-project.org/nosvn/R.check/r-devel-linux-x86_64-fedora-gcc/rcrisp-00check.html

EDITED: sorry, I had a closer look at the failed check above and I thought it was a different check from the one which we are targeting with this PR. However, I think this PR solves the issue with the vignette, but the CRAN check also reveals that we still have problems with tests, if the example data turns out not to be available. Considering the amount of problems we are having with the retrieval of example data as part of CI and CRAN regular checks, I would suggest the following:

  • let's create some very small artificial dataset, which we can add to this repository without licensing issue. We could use simple geometric shapes or use generative AI to produce more realistic datasets. Let's use these datasets in all tests that periodically run on CRAN and on CI (and in examples?). Let's skip on both CRAN and CI all tests and examples that use the example data that we have on Zenodo.

  • Let's freeze the vignettes by using the pre-computing technique linked above. The drawback is that one needs to remember do periodically update the vignettes (e.g. before each release). We could make a list of all tasks to be carried out before the release and add it to the developer's instructions (do we have these? If not, let's create them :) ).

What do you think @cforgaci ? If you agree, I could take care of these tasks within this week.

Co-authored-by: Francesco Nattino <49899980+fnattino@users.noreply.github.com>
@cforgaci
Copy link
Copy Markdown
Contributor Author

What do you think @cforgaci ? If you agree, I could take care of these tasks within this week.

@fnattino many thanks for looking into this! I agree with your proposal. I cannot work on this until the CRAN deadline on 25 November, so it would be great if you can take care of it.

all examples can now run offline
@fnattino
Copy link
Copy Markdown
Contributor

There's a failing test - having a look at it now.

Copy link
Copy Markdown
Contributor

@fnattino fnattino left a comment

Choose a reason for hiding this comment

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

Hi @cforgaci this looks ready to me - let me know if there is anything else I can help with here!

@cforgaci
Copy link
Copy Markdown
Contributor Author

What do you think @cforgaci ? If you agree, I could take care of these tasks within this week.

@fnattino many thanks for looking into this! I agree with your proposal. I cannot work on this until the CRAN deadline on 25 November, so it would be great if you can take care of it.

Thanks, @fnattino! I will update the date in the changelog, merge and resubmit to CRAN later today.

@cforgaci cforgaci merged commit 48f5fe7 into main Nov 24, 2025
8 checks passed
@cforgaci cforgaci deleted the 347-net-vig-http-fail-fix-cf branch November 24, 2025 13:08
@cforgaci
Copy link
Copy Markdown
Contributor Author

@fnattino, FYI, rcrisp 0.3.1 is on CRAN.

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.

Network vignette failing on CRAN

2 participants