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

Suppress spdep warnings #441

Merged
merged 3 commits into from
Sep 16, 2024
Merged

Suppress spdep warnings #441

merged 3 commits into from
Sep 16, 2024

Conversation

r-ash
Copy link
Contributor

@r-ash r-ash commented Sep 16, 2024

Since updating spdep to v 1.3.6 we're seeing more warnings being raised for Naomi test data:
Warning messages:
1: In spdep::poly2nb(sh) : some observations have no neighbours;
if this seems unexpected, try increasing the snap argument.
2: In spdep::poly2nb(sh) : neighbour object has 2 sub-graphs;
if this sub-graph count seems unexpected, try increasing the snap argument.
3: In spdep::mat2listw(abs(Q), style = "B", zero.policy = TRUE) :
neighbour object has 2 sub-graphs

Stack trace

── Warning ('test-03-run-model.R:74:5'): real model can be run ─────────────────
some observations have no neighbours;
if this seems unexpected, try increasing the snap argument.
Backtrace:
     ▆
  1. ├─withr::with_envvar(...) at test-03-run-model.R:73:3
  2. │ └─base::force(code)
  3. └─hintr:::run_model(data, options, tempdir()) at test-03-run-model.R:74:5
  4.   └─naomi::hintr_run_model(data, options, model_output_path, validate = FALSE) at hintr/R/run_model.R:72:3
  5.     ├─naomi:::handle_naomi_warnings(run_model(data, options, validate))
  6.     │ ├─base::withCallingHandlers(...)
  7.     │ └─base::force(expr)
  8.     └─naomi:::run_model(data, options, validate)
  9.       └─naomi:::naomi_prepare_data(data, options)
10.         └─naomi::naomi_model_frame(...)
11.           └─naomi::create_adj_matrix(mf_areas_sf)
12.             ├─naomi:::suppress_one_message(spdep::poly2nb(sh), "although coordinates are longitude/latitude, st_intersects assumes that they are planar")
13.             │ └─base::withCallingHandlers(...)
14.             └─spdep::poly2nb(sh)

This is expected as Likoma island is separate, and it will potentially be super noisy for running tests and the production app. So suppress the warnings for now.

I have done this by updating our suppress_one_* utils into a single suppress_conditions where you can specify the messages and warnings you want to ignore.

Copy link
Collaborator

@jeffeaton jeffeaton left a comment

Choose a reason for hiding this comment

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

Code changes all look good to me so long as tests pass.

I've requested adding link to the new spdep vignette in the NEWS as a memory aide to future me.

Approving now as the change is non-critical and so you don't have to make me look at it again.

NEWS.md Outdated
@@ -1,10 +1,11 @@
# naomi 2.9.28

* Make duckdb an optional dependency
* Suppress "some observations have no neighbours" and "neighbour object has 2 sub-graphs" warnings from `spdep` v1.3.6. We expect this warning for some countries and it will make tests and output noisy to leave on.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add the link to the spdep vignette describing the package change? (for future me when I forget again)

@r-ash r-ash merged commit eefdd19 into master Sep 16, 2024
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