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

Address locked binding issue #309

Merged
merged 4 commits into from
Jun 24, 2022

Conversation

milevavantuyl
Copy link
Collaborator

@milevavantuyl milevavantuyl commented Jun 1, 2022

Updated README instructions and code for loading aif360 functions to address issue #298 about locked binding

Signed-off-by: milevavantuyl <mvantuyl@wellesley.edu>
Signed-off-by: milevavantuyl <mvantuyl@wellesley.edu>
@milevavantuyl milevavantuyl marked this pull request as ready for review June 2, 2022 20:08
@@ -116,6 +116,8 @@ load_aif360_lib()
load_aif360_lib()
```

If you get an error: `cannot change value of locked binding`, please restart the R session and run `load_aif360_lib()` exactly once.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add function to load the library.

Signed-off-by: milevavantuyl <mvantuyl@wellesley.edu>
lockBinding("post_algo", e)
lockBinding("tf", e)
} else {
message(paste0(c("The aif360 functions have already been loaded:", bindings), sep = " "))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Two suggestions.

  • Let's not display all the functions as it is not required for the users
  • Expand the message to something like The aif360 functions have already been loaded. You can begin using the package

Copy link
Collaborator

@SSaishruthi SSaishruthi left a comment

Choose a reason for hiding this comment

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

Can we update the comments?

Signed-off-by: milevavantuyl <mvantuyl@wellesley.edu>
Copy link
Collaborator

@SSaishruthi SSaishruthi left a comment

Choose a reason for hiding this comment

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

lgtm

@SSaishruthi SSaishruthi removed the request for review from gdequeiroz June 23, 2022 23:34
@SSaishruthi SSaishruthi added the R AIF360 R related issues label Jun 23, 2022
@SSaishruthi SSaishruthi merged commit 18f0298 into Trusted-AI:master Jun 24, 2022
Illia-Kryvoviaz pushed a commit to Illia-Kryvoviaz/AIF360 that referenced this pull request Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R AIF360 R related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants