refactor!: New arpack_defaults() replaces igraph.arpack.default and arpack_defaults lists

[maelle]

…arpack_defaults() as a function

Like #762
Fix #714

[maelle]

@krlmlr how can I add this change (changing the default argument but also doing the "hack" inside the function to not break reverse dependencies) to functions of aaa-auto.R?

[krlmlr]

Can you please rephrase?

The checks are failing here.

[maelle]

I have made changes in some scripts manually. For the checks to pass, I'd need to update aaa-auto.R. However I do not know how to do that not manually. I know where aaa-auto.R content comes from but

• the "hack" with rlang is a bit longer than what's in the YAML files.
• I don't know how to update aaa-auto.R based on the YAML file.

[ntamas]

aaa-auto.R is generated from tools/stimulus/aaa-auto.R.in (which is basically empty) using Stimulus. I think it might be easier if you could simply show what you'd like to do in aaa-auto.R and then I can tell you what the easiest option would be.

[maelle]

For functions that have arpack_defaults as a default argument value, I need to change it to arpack_defaults(), but also to add these lines at the top of the function body

  eval_try <- rlang::eval_tidy(options)
  options_value <- rlang::call_args(rlang::current_call())[["options"]]
  if (is(eval_try, "function") && as.character(options_value) == "arpack_defaults") {
    lifecycle::deprecate_soft(
      "1.5.0",
      I("arpack_defaults"),
      "arpack_defaults()",
      details = c("So the function arpack_defaults(), not an object called arpack_defaults.")
    )
    options <- arpack_defaults()
  }

I'll explore a bit more.

[krlmlr]

Thanks. Some functions still seem to use igraph.arpack.default, according to check results?

[krlmlr]

Why eval_tidy()? We should be able to do this with standard evaluation, forcing should be enough?

[maelle]

do you have any example I could look at?

[krlmlr]

Numbers are cheap.

[maelle]

what do you mean?

[maelle]

yes because I'm stuck, not knowing how to add the lines to the body of functions that are created automatically #800 (comment) (I won't get back to this this week)

[krlmlr]

Oh, got you. Instead of changing the body of an autogenerated function, can we wrap it instead, as done in #859?

[maelle]

In this PR I have updated types-RR.yaml, however running the makefile doesn't update aaa-auto.R, I am confused.

@krlmlr good idea, will do.

[ntamas]

In this PR I have updated types-RR.yaml, however running the makefile doesn't update aaa-auto.R, I am confused.

Are you running Makefile-cigraph? This is the makefile that contains the rule that updates aaa-auto.R.

[maelle]

I ran make -f Makefile-cigraph src/rinterface.c R/aaa-auto.R as indicated in tools/README.md

[maelle]

But maybe I should use other arguments actually? Could that be documented in that README? I'd happily test the instructions. 😸

[krlmlr]

I'll tweak here too.

[aviator-app]

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.

---

See the real-time status of this PR on the Aviator webapp.

Use the Aviator Chrome Extension to see the status of your PR within GitHub.

[krlmlr]

@maelle: Can you please take a look at the failing tests? Do you need more support?

[maelle]

My problems here are

• not knowing where to change the code for aaa-auto.R to be edited properly.
• not understanding the comments refactor!: New arpack_defaults() replaces igraph.arpack.default and arpack_defaults lists #800 (comment) and refactor!: New arpack_defaults() replaces igraph.arpack.default and arpack_defaults lists #800 (comment)

[maelle]

thanks @krlmlr