Rename resolution_parameter to resolution

[vtraag]

Fixes #883. I've only updated the R file and the test, all the rest should follow automatically, if my understanding is correct? If more is needed, let me know.

[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 manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.

---

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.

[maelle]

I think we need to temporarily allow the two argument names? https://lifecycle.r-lib.org/articles/communicate.html#renaming-an-argument

[vtraag]

Sure, sounds like a good idea!

I'm not entirely sure why the checks fail. Are the examples from the documentation not updated before being run or something? Should I run something in order to trigger an update of the docs?

[maelle]

@vtraag ah yes you need to run devtools::document() to update the Rd files (the manual pages).

[maelle]

the version number here should be 2.0.0 (I think?) to go with the next release. it's only confusing for those who'll install the dev version.

[maelle]

should it be lifecycle::deprecate_soft() for this version?

[vtraag]

the version number here should be 2.0.0 (I think?) to go with the next release. it's only confusing for those who'll install the dev version.

Why would this be confusing? Happy to say 2.0.0 here, but when we release any pre-2.0 release with this in it, it would be incorrect, and might be more confusing. (I now realise that I might have targeted the dev branch due to breaking changes, but they're now no longer breaking). Or am I misunderstanding something here?

[vtraag]

@vtraag ah yes you need to run devtools::document() to update the Rd files (the manual pages).

Unfortunately can't do that now, apparently have to install something still, but R mirrors seem to be down:

[vtraag]

I've now updated the docs.

By the way, I'm not sure how the deprecation should be dealt with in the docs? Should the old argument still be there, saying it's deprecated, and point to the new argument? Are there any defaults for this in R (sorry, I almost never work in R)?

[vtraag]

Feeling like a complete n00b again! Now get Error: Error in deprecated() : argument "old" is missing, with no default in the CI. According to the docs deprecated doesn't have any arguments: https://lifecycle.r-lib.org/reference/deprecated.html. So I'm not sure what the error means or what can be done about it. Any suggestions @maelle ?

[krlmlr]

I've seen this before, igraph also implements a deprecated() function. Can we rename (in a separate PR), to avoid this kind of confusion?

[vtraag]

Can we for now use lifecycle::deprecated ? What is the igraph::deprecated function used for, and do we need to retain it at all? (Separate PR and/or issue might be good indeed).

[maelle]

@vtraag the igraph deprecated() function will soon be removed, by #1014 and #1104 which aren't ready yet (and thanks to other commits).

[krlmlr]

@maelle: I forgot why this is assigned to me. We probably want to add an ellipsis after the second argument and run revdepchecks. Can you please help with the ellipsis?

[maelle]

Sure, I'll do that, but what's our rationale for choosing to put check_dots_empty() in auto-generated vs wrapper code?

[maelle]

the existence of igraph's own deprecated() remains problematic but I used what we did in another function

[krlmlr]

Nice! Good to go from my end.

[krlmlr]

This now needs to be 2.0.x