luukvdmeer / sfnetworks

Tidy Geospatial Networks in R
https://luukvdmeer.github.io/sfnetworks/
Other
352 stars 20 forks source link

Update agr.R file adding a new function is_null_active() #253

Open SunSummoner opened 1 year ago

SunSummoner commented 1 year ago

Added a new function is_null_active() for the if loop and removed the if loops. Also, added statement for using library thePackages for using the function. Not sure if it is necessary for the main code.

agila5 commented 1 year ago

Hi @SunSummoner and thanks for your PR. A couple of comments:

  1. You cannot use install.packages or library in an R script that must be included in an R package.
  2. What is "thePackage"?
  3. What are the benefits of your approach with respect to the existing code? Moreover, I'm not convinced that your code is working since you are not overwriting the value of the active object.
SunSummoner commented 1 year ago

Hi @SunSummoner and thanks for your PR. A couple of comments:

  1. You cannot use install.packages or library in an R script that must be included in an R package.
  2. What is "thePackage"?
  3. What are the benefits of your approach with respect to the existing code? Moreover, I'm not convinced that your code is working since you are not overwriting the value of the active object.

Hi @agila5 thank you for your comments. I actually wasn't sure if you can add packages in R script. Thank you for clarifying that. thePackage is the package is used to using the function I wrote as it was showing error that it couldn't find the function. The benefit I saw in my code was reducing the redundancy of the if loop. When I ran the code it was working but I understand your concern. I wanted to started contributing to this repo. Are there any issues you think I can work on apart from this PR?