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

Add angle as filter #529

Closed
wants to merge 12 commits into from

Conversation

wangzhao0217
Copy link
Collaborator

Hi Robin,
Sorry I forgot to create the PR to let you review.
Please have a look at this latest version with the following changes:

  • The angle mask is moved to rnet_join function as an argument, rnet_merge function is unchanged.
  • I tried to use the angle_diff function to replace calculate_angle/get_vector, but the results do not look quite right and are running slow (It may not be caused by the angle_diff function, but rather by the way I am using it.).

Here is the result.
image

@Robinlovelace
Copy link
Member

Robinlovelace commented Sep 13, 2023

Looks good, thanks @wangzhao0217 . Problem: GitHub Actions are failing. Can you test on this branch with

devtools::check()

?

If you pull down the latest version from the master branch you can test it works: please confirm devtools::check() works on the master branch, that should help debug any issues that are making the actions fail, as illustrated by the red x symbols above.

@Robinlovelace
Copy link
Member

Update on this: I've had a look to try to diagnose the issue. It seems that you are calling functions from the {devtools} package in the vignette. Can you try removing reference to the package in the vignetter, or at least setting eval=FALSE in the relevant code chunk?

See here: https://github.com/ropensci/stplanr/actions/runs/6171414373/job/16749592822#step:6:46

@@ -17,11 +17,12 @@ knitr::opts_chunk$set(
message = FALSE,
warning = FALSE
)
devtools::load_all()
# devtools::load_all()
Copy link
Member

Choose a reason for hiding this comment

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

👍 that should solve it

@Robinlovelace
Copy link
Member

Hi @wangzhao0217 here's another suggestion that should be a quick fix to get actions passing:

devtools::document()

@Robinlovelace
Copy link
Member

Closed in favour of #530

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