Skip to content

Dev#49

Open
spearmangillman wants to merge 19 commits into
mainfrom
dev
Open

Dev#49
spearmangillman wants to merge 19 commits into
mainfrom
dev

Conversation

@spearmangillman
Copy link
Copy Markdown
Member

@spearmangillman spearmangillman commented May 8, 2026

Script Organization

  • Descriptive header (includes name, affiliation, date, brief description of script purpose)
  • Parameters specified at top of script
  • Files loaded at top of script
  • Functions abstracted into separate script
  • Organized into sections

Readability

  • Repetitive code refactored into functions
  • All functions have docstrings
  • Descriptive in-line comments
  • Correct casing for variables and functions: camelCase (R) or snake_case (Python)
  • Descriptive variable and function names – variables should be nouns and functions should be verbs
  • Used a linter

Generalizability

  • No hardcoded values (all integers, strings, etc. should be specified at the top of the script in the “Parameters” section)
  • Use relative file paths
  • Consistent output file naming (e.g. “example-filename.tif”)

Summary by CodeRabbit

  • New Features

    • Headless Unix support for interactive apps (runs without GUI and shows local access instructions).
    • Package version bumped to 2.5.1.
  • Documentation

    • Clarified Background Data Generation behavior and preservation option.
    • Noted MESS/MoD maps apply only to continuous variables.
    • Updated minimum SyncroSim requirement to 3.1.27+.
  • Improvements

    • Cross-platform path and raster handling, PNG rendering on Unix, more robust MaxEnt/Java invocation, explicit app naming, and updated runtime environment specifications.

Review Change Stack

spearmangillman and others added 17 commits April 1, 2026 16:01
- Replace Windows batch file approach with direct system2("java") invocation in new runMaxent() helper; works cross-platform, no .bat file needed.
- Update checkJava() to resolve Java via JAVA_HOME and common install directories when SyncroSim's minimal PATH does not include Java, patching the R session PATH so all subsequent calls succeed.
- Remove fsep = "\\" from file.path() calls used by R file operations.
- Clean up section organization in 08-fit-model-functions.R.
Split the single shared conda environment into separate Python and R environments to allow independent dependency management and avoid conflicts between the two runtimes.
updated 00-constants.R to set bitmapType = "cairo" on Unix when Cairo is available, removing the X11 display dependency for PNG output in R transformers.
Explicitly set blockxsize=256, blockysize=256 when writing tiled template rasters to ensure valid GeoTIFF tiling on Linux.
- Updated launchShinyApp to detect headless Unix environments (no DISPLAY) and bind to 0.0.0.0:3838 with launch.browser = FALSE, making the interactive viewers accessible via port forwarding or SSH tunnel.
- Added an appName parameter so the live status message identifies the accessible app
…t use adehabitatHR library"

This reverts commit 4e7f0b7.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Note

.coderabbit.yml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'code_guidelines', 'ai'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: c5b1b467-1ce8-40dd-86ab-864cc5c4928e

📥 Commits

Reviewing files that changed from the base of the PR and between b5f393a and 0ccaabd.

📒 Files selected for processing (1)
  • src/08-fit-model-functions.R
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/08-fit-model-functions.R

📝 Walkthrough

Walkthrough

Refactors MaxEnt to invoke Java directly, patches Java PATH, adds headless-Unix Shiny launch and app naming, standardizes cross-platform paths/tiling, splits Conda envs into Python/R (v6), bumps package to 2.5.1, and updates documentation about background data and MESS/MoD.

Changes

WISDM 2.5.1 Release

Layer / File(s) Summary
Environment Configuration & Constants
src/00-constants.R, src/wisdm-py-conda.yml, src/wisdm-r-conda.yml, src/wisdm-conda.yml
Sets options(bitmapType = "cairo") on Unix when available; original wisdm-conda.yml cleared and replaced by separate Python and R Conda specs.
Shiny Launch & Java PATH Handling
src/00-helper-functions.R
launchShinyApp() gains appName; headless-Unix branch runs Shiny on 0.0.0.0:3838 without browser; checkJava() patches R session PATH when java is missing.
MaxEnt Invocation & fitModel Refactor
src/08-fit-model-functions.R
fitModel()'s maxent branch now calls runMaxent() which invokes Java via system2; lambda loading paths updated for full vs CV fits; batch-file workflow removed; CV messaging reformatted; many model helper section headers added.
Cross-Platform Path Handling & Tiling
src/04-background-data-functions.R, src/8-fit-maxent.R, src/8-fit-glm.R, src/10-ensemble-model.py, src/2-spatial-data-preparation.py
Switched to file.path()/POSIX-style separators, removed unconditional slash gsub and Windows batch outputs, and set raster blockxsize/blockysize to 256.
Shiny App Naming
src/6-variable-reduction.R, src/7-hyperparameter-tuning.R
Calls to launchShinyApp() now include explicit appName values for UI messaging.
Multiprocessing Configuration
src/9-apply-model.R
Adds desync_max_sec = 10 to setup_session() in Partial library branch.
Pipeline Configuration & Version Bump
src/package.xml
Package version bumped to 2.5.1; transformers updated to use wisdm-py-conda.yml or wisdm-r-conda.yml with condaEnvVersion="6".
Documentation
docs/_scenarios/section2.md, docs/_scenarios/section3.md, docs/_scenarios/section6.md, docs/index.md
Formatting and explanatory notes added: Background Data Generation formatting, background-site regeneration behavior on re-run, MESS/MoD limited to continuous variables, and SyncroSim requirement updated to 3.1.27+.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested reviewers

  • katieb1
  • afilazzola

Poem

🐰 I hopped through slashes, cairo in my pack,
Java called directly — no .bat on my back.
Conda split tidy, R and Python in line,
Shiny wakes headless, named apps: all fine.
A small carrot for docs, the release looks divine.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Dev' is vague and generic, providing no meaningful information about the changeset's primary purpose or scope. Replace with a descriptive title summarizing the main changes, such as 'Refactor platform-specific paths, improve Shiny app handling, and update conda environments' or similar.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/00-helper-functions.R (1)

629-643: ⚡ Quick win

Make the headless Shiny port configurable.

Line 629 hardcodes port 3838, which can cause startup failures when multiple jobs run on the same host. Allow an env override to reduce collisions.

Suggested patch
-    port <- 3838
+    port <- suppressWarnings(as.integer(Sys.getenv("WISDM_SHINY_PORT", "3838")))
+    if (is.na(port) || port < 1L || port > 65535L) {
+      port <- 3838L
+    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/00-helper-functions.R` around lines 629 - 643, The port is hardcoded to
3838; change the code that sets port (the port variable used before progressBar
and shiny::runApp) to read an environment override (e.g.,
Sys.getenv("SHINY_PORT") or similar), coerce it to an integer with a safe
fallback to 3838 when missing/invalid, and use that variable in the progressBar
message (which uses appName) and in shiny::runApp(appDir = appPath, host =
"0.0.0.0", port = port, launch.browser = FALSE); ensure invalid/NA env values
are handled so the fallback 3838 is used.
src/08-fit-model-functions.R (1)

636-636: 💤 Low value

Consider validating maxent.jar existence before invocation.

If maxent.jar is missing from the package directory, the Java invocation will fail with a generic "jarfile not found" error. An early check would provide a clearer diagnostic.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/08-fit-model-functions.R` at line 636, Check that the computed jarPath
(jarPath <- file.path(ssimEnvironment()$PackageDirectory, "maxent.jar"))
actually exists before calling Java; if file.exists(jarPath) is FALSE, stop with
a clear diagnostic (e.g., stop("maxent.jar not found in package directory: ",
jarPath)) so callers get an explicit error rather than a generic "jarfile not
found" from Java.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/08-fit-model-functions.R`:
- Around line 692-694: The call to system2("java", args = args) ignores its exit
status so Java/MaxEnt failures go unnoticed; capture the return value from
system2 (e.g., exit_code <- system2(..., stdout=..., stderr=...)) and check for
non-zero, then stop() or throw an informative error that includes the exit code
and any captured stdout/stderr before proceeding to read.maxent and expecting
species.lambdas; update the block that builds args and invokes system2 to
perform this check and fail fast with a clear message referencing system2, args,
and read.maxent/species.lambdas.

---

Nitpick comments:
In `@src/00-helper-functions.R`:
- Around line 629-643: The port is hardcoded to 3838; change the code that sets
port (the port variable used before progressBar and shiny::runApp) to read an
environment override (e.g., Sys.getenv("SHINY_PORT") or similar), coerce it to
an integer with a safe fallback to 3838 when missing/invalid, and use that
variable in the progressBar message (which uses appName) and in
shiny::runApp(appDir = appPath, host = "0.0.0.0", port = port, launch.browser =
FALSE); ensure invalid/NA env values are handled so the fallback 3838 is used.

In `@src/08-fit-model-functions.R`:
- Line 636: Check that the computed jarPath (jarPath <-
file.path(ssimEnvironment()$PackageDirectory, "maxent.jar")) actually exists
before calling Java; if file.exists(jarPath) is FALSE, stop with a clear
diagnostic (e.g., stop("maxent.jar not found in package directory: ", jarPath))
so callers get an explicit error rather than a generic "jarfile not found" from
Java.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 1a83b5f8-1e62-4563-a2be-b01b909ad003

📥 Commits

Reviewing files that changed from the base of the PR and between b8b7c8d and ffa4489.

📒 Files selected for processing (19)
  • docs/_scenarios/section2.md
  • docs/_scenarios/section3.md
  • docs/_scenarios/section6.md
  • docs/index.md
  • src/00-constants.R
  • src/00-helper-functions.R
  • src/04-background-data-functions.R
  • src/08-fit-model-functions.R
  • src/10-ensemble-model.py
  • src/2-spatial-data-preparation.py
  • src/6-variable-reduction.R
  • src/7-hyperparameter-tuning.R
  • src/8-fit-glm.R
  • src/8-fit-maxent.R
  • src/9-apply-model.R
  • src/package.xml
  • src/wisdm-conda.yml
  • src/wisdm-py-conda.yml
  • src/wisdm-r-conda.yml
💤 Files with no reviewable changes (1)
  • src/wisdm-conda.yml

Comment thread src/08-fit-model-functions.R Outdated
coderabbit update

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/08-fit-model-functions.R`:
- Around line 600-606: The Windows branch in checkJava (the if (os == "Windows")
block) incorrectly uses shell() with ignore.stdout/ignore.stderr which shell()
doesn't support, causing checkJava to fail on Windows; replace that shell(...)
call with system2("java", args = "-version", stdout = FALSE, stderr = FALSE) so
the Java probe suppresses output correctly and mirrors the non-Windows check,
ensuring checkJava and downstream MaxEnt detection work on Windows.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 6a479d62-79e4-4e29-be4f-77bcf02a120e

📥 Commits

Reviewing files that changed from the base of the PR and between ffa4489 and b5f393a.

📒 Files selected for processing (1)
  • src/08-fit-model-functions.R

Comment thread src/08-fit-model-functions.R Outdated
Replaced platform-specific Java version check with a unified system2 call.
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.

2 participants