Skip to content

Conversation

@rpreen
Copy link
Collaborator

@rpreen rpreen commented Nov 7, 2025

@JessUWE this adds quite a few markdown files to the root directory - could they be moved to a docs folder?

Pre-commit needs to be run to clean up.

This warning needs to be fixed:

* checking files in ‘vignettes’ ... WARNING
Warning: Files in the 'vignettes' directory but no files in 'inst/doc':
  ‘example-notebook.Rmd’

I think changing the R-CMD-check.yaml as follows might fix it:

FROM:

      - name: Check R Package
        uses: r-lib/actions/check-r-package@v2
        with:
          upload-snapshots: true

TO:

      - name: Check R Package
        uses: r-lib/actions/check-r-package@v2
        with:
          upload-snapshots: true
          build-vignettes: true

Add documentation for ACRO and SACRO tools with features and installation instructions.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Updated the installation guide to include installation instructions for the ACRO package from CRAN and GitHub, along with core features and prerequisites.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
This document outlines the capabilities of ACRO, including supported data analysis functions, programming languages, disclosure control features, and integration capabilities.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
This file provides an overview of the ACRO-R package, its purpose, usage, and acknowledgments.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Added examples and tutorials for using ACRO in R.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Created a figures folder to hold the SACRO_Logo within the Man folder

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Added light-switch feature and updated navbar structure.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Created vanilla CSS and JS files to style the documentation page and lightswitch feature.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Removed concurrency restrictions and environment variables from pkgdown workflow.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Updated the link for SACRO-Viewer documentation.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
@codecov
Copy link

codecov bot commented Nov 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (431f718) to head (1ae1d12).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #30   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          102       107    +5     
=========================================
+ Hits           102       107    +5     

☔ 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.

Removed PDF reports and HTML summaries from output formats.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
@jim-smith
Copy link
Collaborator

@JessUWE the package repository CRAN won't accept having many files in the root directory - they need to be in a directory called "inst"
If you look at the current main branch you can see that I've had to move the images folder and installation.md into inst/ and change some of the links in the main readme.md file.
I've sent those changed versions to CRAN so you should probably pull those changes into this branch to keep things consistent

JessUWE and others added 4 commits November 13, 2025 19:19
Fix merging issues and CRAN
…ut management. Fixed broken links to documentation;

ACRO.sacro-tools.org
SACRO-ML.sacro-tools.org
ACRO-R.sacro-tools.org
SACRO-Viewer.sacro-tools.org
Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
@JessUWE
Copy link
Contributor

JessUWE commented Nov 13, 2025

@rpreen Can you check please?

JessUWE and others added 4 commits November 13, 2025 20:49
…te DESCRIPTION for Roxygen version

Update pre-commit configuration, fix links in documentation, and update DESCRIPTION for Roxygen version
Updated Python version requirement from 3.8 to 3.10.

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Copy link
Collaborator Author

@rpreen rpreen left a comment

Choose a reason for hiding this comment

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

Looks like there are a lot of files in here that shouldn't be:

  • .vscode/ - I don't think editor configs belong in this repo
  • vignettes/ contains .swp files that should be removed - add .swp to gitignore
  • .Rdata is a binary file - why is it needed?
  • docs/ contains a lot of html files - are we expected to edit/maintain these? if they are generated then why aren't they created and pushed to the gh-pages branch?

@rpreen
Copy link
Collaborator Author

rpreen commented Nov 13, 2025

The errors and warnings in the R-CMD-check runners need to be addressed.

Non-standard files/directories found at top level:
  ‘docs’ ‘examples.md’ ‘index.md’ ‘installation.md’ ‘pkgdown’
  ‘supports.md’ ‘welcome.md’

Things that CRAN should ignore need to be added to .Rbuildignore - most of the markdown files (examples.md, installation.md, etc.) should really be moved to a subfolder to keep the root clean - the inst/ directory as Jim suggested is one suitable place?

< Potential spelling errors:
<   WORD           FOUND IN
< GPLv           index.md:37
< NK             index.md:29
< PyPI           index.md:38
< Scalable       index.md:43
< TRE            index.md:9
< btn            index.md:7,7,9,9,49,49,49
< crosstab       index.md:39
< finalise       index.md:31
< mitigations    index.md:30
< organisation   index.md:42
< sacro          index.md:7,9
< sm             index.md:7,7,9,9
< statsmodels    index.md:28

These may go away if these files are ignored by CRAN, but otherwise I believe the words need to get added to the ignore file inst/WORDLIST

@JessUWE
Copy link
Contributor

JessUWE commented Nov 13, 2025

The errors and warnings in the R-CMD-check runners need to be addressed.

Non-standard files/directories found at top level:
  ‘docs’ ‘examples.md’ ‘index.md’ ‘installation.md’ ‘pkgdown’
  ‘supports.md’ ‘welcome.md’

Things that CRAN should ignore need to be added to .Rbuildignore - most of the markdown files (examples.md, installation.md, etc.) should really be moved to a subfolder to keep the root clean - the inst/ directory as Jim suggested is one suitable place?

< Potential spelling errors:
<   WORD           FOUND IN
< GPLv           index.md:37
< NK             index.md:29
< PyPI           index.md:38
< Scalable       index.md:43
< TRE            index.md:9
< btn            index.md:7,7,9,9,49,49,49
< crosstab       index.md:39
< finalise       index.md:31
< mitigations    index.md:30
< organisation   index.md:42
< sacro          index.md:7,9
< sm             index.md:7,7,9,9
< statsmodels    index.md:28

These may go away if these files are ignored by CRAN, but otherwise I believe the words need to get added to the ignore file inst/WORDLIST

@rpreen Yes, I can do that. Hopefully, it will help. I’ve been dealing with several errors in Acro-R since I downloaded it. It’s quite different from a regular Python project. Could you please review the content when you have time? It will be going live soon, and I’d like to finalize everything by tomorrow if possible.

@JessUWE
Copy link
Contributor

JessUWE commented Nov 14, 2025

@rpreen

@rpreen
Copy link
Collaborator Author

rpreen commented Nov 14, 2025

Looking... it seems to be that the test for acro_glm() is not being executed. Revert your changes to test-acro_glm.R?‎

Please:

  • move NEWS.md back to the root folder
  • remove the .swp file in vignettes/

JessUWE and others added 3 commits November 15, 2025 09:28
Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Added https to the SACRO-ML link

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
@JessUWE
Copy link
Contributor

JessUWE commented Nov 26, 2025

Looking... it seems to be that the test for acro_glm() is not being executed. Revert your changes to test-acro_glm.R?‎

Please:

  • move NEWS.md back to the root folder
  • remove the .swp file in vignettes/

@rpreen I think everything has passed now. However, I had to replace skip_on_ci() with skip_on_cran() in the file, which was why it wasn’t working before. It should be fine now.

Could I please ask why we’re moving the NEWS.md file back to the root folder? I thought it was supposed to be organized according to CRAN standards.

@rpreen
Copy link
Collaborator Author

rpreen commented Nov 26, 2025

Could I please ask why we’re moving the NEWS.md file back to the root folder? I thought it was supposed to be organized according to CRAN standards.

It is standard practice to place LICENSE, README, and CHANGELOG files such as NEWS.md in the root directory of repositories so they are immediately accessible. For example, see the popular dplyr and ggplot2 R packages. It has been in the root directory since the start and gets linked correctly on CRAN

uses: actions/checkout@v4
- name: Setup Python
uses: actions/setup-python@v6
uses: actions/setup-python@v5
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You've downgraded the setup-python action from v6 to v5 - is this necessary because we will need to watch for the bot attempting to autoupdate it later?

@JessUWE
Copy link
Contributor

JessUWE commented Nov 26, 2025

Could I please ask why we’re moving the NEWS.md file back to the root folder? I thought it was supposed to be organized according to CRAN standards.

It is standard practice to place LICENSE, README, and CHANGELOG files such as NEWS.md in the root directory of repositories so they are immediately accessible. For example, see the popular dplyr and ggplot2 R packages. It has been in the root directory since the start and gets linked correctly on CRAN

@rpreen I understand the purpose of the license, README, and changelog files. I was just curious about the NEWS.md file maybe because it’s an R project, I expected it to contain more, so it looked a bit unusual to me, which is why I asked. But it has now been moved to the root folder

nursery_data$recommend[which(nursery_data$recommend == "priority")] <- "3"
nursery_data$recommend[which(nursery_data$recommend == "spec_prior")] <- "4"
nursery_data$recommend <- as.numeric(nursery_data$recommend)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there a reason for removing the blank lines between functions in test-acro_glm.R because it's harder to read now? All the other test files follow the convention of having a blank line between them.

Since there are no other changes to this file, I suggest reverting the file completely.

r-version: ${{ matrix.r }}

- name: Install Python ACRO
run: python -m pip install acro
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure that this is necessary to add on the CI because reticulate is installing the Python package in the virtual environment that is used.

Copy link
Contributor

Choose a reason for hiding this comment

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

@rpreen done! Sorry it took time, had errors and had to do a weird reversal on the code!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you quickly just try removing the pip install acro from the yaml file too? I believe that is doing a system install but reticulate uses its own virtual environment so it's not needed.

Copy link
Contributor

@JessUWE JessUWE Nov 27, 2025

Choose a reason for hiding this comment

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

@rpreen ya done!

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
I've removed the - name: Install Python ACRO step that was running python -m pip install acro

Signed-off-by: Jessica Ikechukwu <Jessica.Ikechukwu@uwe.ac.uk>
@rpreen
Copy link
Collaborator Author

rpreen commented Nov 27, 2025

thanks, lgtm

@rpreen rpreen merged commit ddb5da0 into AI-SDC:main Nov 27, 2025
10 checks passed
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