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

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SunSummoner
Copy link

@SunSummoner SunSummoner commented Aug 11, 2023

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.

I added a new function is_null_active() to remove the if loop in code.
Update agr.R added function is_null_active
Removed the return statement
@agila5
Copy link
Collaborator

agila5 commented Aug 11, 2023

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
Copy link
Author

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?

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