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

feat: max_degree() #1403

Merged
merged 1 commit into from
Jul 2, 2024
Merged

feat: max_degree() #1403

merged 1 commit into from
Jul 2, 2024

Conversation

szhorvat
Copy link
Member

@szhorvat szhorvat commented Jun 16, 2024

Please see the chatroom before you comment here.

This adds max_degree(), and is also meant to serve as an example for beginner contirbutors on how to expose a new function.

Why do we want max_degree() instead of just using max(degree())? This function produces 0 instead of -Inf for empty vertex sets, which is convenient when using it as a building block.

There was a documentation item for \dots for degree(), which did not seem to make sense. I removed it.

This is blocked on the decision-making issue #853: do we use v or vids?


Explanation for beginners on how this PR was produced:

  • igraph_maxdegree() was (re-)enabled in functions-R.yaml
  • PARAM_NAMES can be used to rename parameters from the upstream functions.yaml to fit the standards used in the R interface
  • In this case, PARAM_ORDER is used not to reorder parameters, but to add the ... parameter to the R interface (denoted by * in the interface definition). In the R interface, parameters following the ... must be spelt out with full name.
  • Notice that since ... was added, we need @inheritParams rlang::args_dots_empty in the documentation
  • Command to automatically generate the interface: make -f Makefile-cigraph src/rinterface.c R/aaa-auto.R. The two files mentioned in this command are automatically generated. The generated R function (maxdegree_impl) can either be wrapped by the final public function (max_degree()) or in simple cases (like here) just assigned to it.
  • We must also run R -q -e 'cpp11::cpp_register()' since a new C function was added.
  • We must also run R -q -e 'devtools::document()' to (re-)generate the documentation files in man as well as the NAMESPACE file.
  • Please add an example in the documentation block using #' @examples
  • Don't forget to add tests for the new function, and run the test suite using devtools::test() before submitting the PR.

@szhorvat szhorvat requested review from krlmlr and maelle June 16, 2024 14:19
Copy link
Contributor

aviator-app bot commented Jun 16, 2024

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.

@szhorvat szhorvat force-pushed the feat/maxdegree branch 2 times, most recently from 1e23420 to f1ca4e7 Compare June 16, 2024 14:32
R/aaa-auto.R Show resolved Hide resolved
R/aaa-auto.R Show resolved Hide resolved
R/structural.properties.R Outdated Show resolved Hide resolved
R/structural.properties.R Show resolved Hide resolved
@maelle

This comment was marked as resolved.

@szhorvat szhorvat added this to the 2.0.4 milestone Jun 20, 2024
@szhorvat szhorvat force-pushed the feat/maxdegree branch 2 times, most recently from 22abf24 to b48ece2 Compare June 20, 2024 15:17
@szhorvat
Copy link
Member Author

Resolved conflicts and rebased on main.

@winnieywu

This comment was marked as off-topic.

@aviator-app aviator-app bot merged commit 80b37bf into main Jul 2, 2024
11 checks passed
@aviator-app aviator-app bot deleted the feat/maxdegree branch July 2, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants