-
-
Notifications
You must be signed in to change notification settings - Fork 201
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
chore: use 'parent' instead of 'father' in bfs()
and dfs()
#1523
base: main
Are you sure you want to change the base?
Conversation
Current Aviator status
This pull request is currently open (not queued). How to mergeTo merge this PR, comment
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.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just renames father
to parent
, right? I don't think my review is needed here (I wanted to do this for a long time).
This way you saw it's not even a breaking change 😸 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
R/structural.properties.R
Outdated
@@ -2150,6 +2155,13 @@ bfs <- function( | |||
} | |||
} | |||
|
|||
if (lifecycle::is_present(father)) { | |||
lifecycle::deprecate_warn("2.0.4", "bfs(father)", "bfs(parent)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surprisingly, this code works, but the following is a bit more correct because parent
and father
are argument names:
lifecycle::deprecate_warn("2.0.4", "bfs(father)", "bfs(parent)") | |
lifecycle::deprecate_warn("2.0.4", "bfs(father = )", "bfs(parent = )") |
pred = TRUE, | ||
succ = TRUE, | ||
dist = TRUE | ||
) | ||
}) | ||
}) | ||
|
||
test_that("bfs() deprecated argument", { | ||
test_that("bfs() deprecated arguments", { | ||
g <- graph_from_literal(a - +b - +c, z - +a, d) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#673 would take care of the style here. Can we somehow add this to the package (with .Rbuildignore
) and configure/document the usage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix #880
It's not breaking anything as the functions still also return the "father" field.