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

Fixing bug when a config only points to other config files and Switch to using simple glob instead of regexp #119

Merged
merged 8 commits into from
Dec 2, 2024

Conversation

Troejelsgaard
Copy link
Contributor

@Troejelsgaard Troejelsgaard commented Nov 25, 2024

(1) Fixing a bug that happens when a config only points to other config files, and (2) Switch to using simple glob instead of regexp (ignore the weird branch name that has nothing to do with was actually done)

First, the pull request fixes a bug where the queue was not returned correctly when a config files only points to other config files.

Secondly, the use of glob2rx() had some strange behaviours when trying to point to files with different extensions for example r vs R. For example when using "program.r|R" all R files in the folder was found and when using "program.[rR]" the "[" was not translated correctly.
Therefore, instead of using glob2rx() the code now only uses Sys.glob() The program read_regexp() has as consequence been renamed to read_glob() (and so has the function within this program)

@Troejelsgaard Troejelsgaard changed the title Fixing bug when a config only points ot other config files and Switch to using simple glob instead of regexp Fixing bug when a config only points to other config files and Switch to using simple glob instead of regexp Nov 25, 2024
@akselthomsen akselthomsen self-requested a review November 25, 2024 12:33
Merge branch 'main' into feat_redirect_logs

# Conflicts:
#	NEWS.md
#	R/internal_run.R
#	inst/examples/demo/config_to_config.yaml
#	inst/examples/demo/demo_2_whirl.yml
#	inst/examples/demo/demo_skip_whirl.yaml
#	inst/examples/demo/demo_whirl.yaml
#	tests/testthat/test-read_regexp.R
Copy link

Github pages

Review the pkgdown webpage for the PR here

Copy link

Code coverage

Name Coverage (%)
whirl 59.66
R/approvedpkgs.R 0.00
R/custom_logging.R 63.04
R/enrich_input.R 100.00
R/internal_run.R 100.00
R/mdformats.R 0.00
R/normalize_with_base.R 100.00
R/pb_script.R 0.00
R/quarto.R 100.00
R/read_glob.R 100.00
R/render_summary.R 93.88
R/renv.R 0.00
R/run.R 92.00
R/session.R 73.55
R/status.R 100.00
R/strace.R 0.00
R/use_whirl.R 100.00
R/util_queue_summary.R 100.00
R/utils.R 63.41
R/whirl_queue.R 95.54
R/whirl_r_session.R 76.38
R/zzz.R 0.00

@Lovemore-Gakava Lovemore-Gakava self-requested a review December 2, 2024 07:12
@Lovemore-Gakava
Copy link
Collaborator

@Troejelsgaard and @akselthomsen thanks for the PR!

Checked: Added support for logging of Python scripts with run().
Observations: Python Packages in the session info returns NULL although in the example script I have imported numpy and matplotlib - is this expected?

General: when a wildcard is used to run scripts in a folder, run (tries) to execute all the files in that folder regardless of file format. Lets restrict it to suported files (R, Rmd, Py, qmd)

@Troejelsgaard Troejelsgaard merged commit 9310cc0 into main Dec 2, 2024
13 of 14 checks passed
@Troejelsgaard Troejelsgaard deleted the feat_redirect_logs branch December 2, 2024 12:43
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