privML / privacy-evaluator

The privML Privacy Evaluator is a tool that assesses ML model's levels of privacy by running different attacks on it.
MIT License
17 stars 17 forks source link

Switch to local Imports #111

Closed blauertee closed 3 years ago

blauertee commented 3 years ago

I've looked at some other python repos and people seem to never do import full_package.module but instead import .module. And @adrinjalali also recommended to use local imports. So maybe that's just a small thing we can change before we start into the next sprint.

jtorhoff commented 3 years ago

Also think that relative imports is the way to go. Absolute imports only for tests and everything else outside the package.

blauertee commented 3 years ago

@ team2 are you also fine with changing all the imports?

travela commented 3 years ago

Hey, yes, we are fine with relative imports and we will change our files accordingly. However, as of now the relative imports do not work for me. The only way for me to make any privacy_evaluator imports work is executing everything from the root directory of the project (this is not necessary for those in our team who use PyCharm, but when working from the terminal or VSC it is).

Regardless of that, if I now run membership_inference.py I get the following import error:

    from ...metrics.basics import (
ValueError: attempted relative import beyond top-level package
blauertee commented 3 years ago

@travela can you be more specific what exactly did you do to get that ImportError?

travela commented 3 years ago

@blauertee In the terminal from the root directory of this repository I do the following: $ git checkout upstream/team1sprint3 $ git pull upstream team1sprint3 $ pip install . $ python3 privacy_evaluator/attacks/membership_inference/membership_inference.py

Which then produced the error:

 File "privacy_evaluator/attacks/membership_inference/membership_inference.py", line 4, in <module>
     from ...metrics.basics import (
 ValueError: attempted relative import beyond top-level package
jtorhoff commented 3 years ago

Running python files that contain relative imports from the command line is generally possible but involves some work on top, it's a well known issue and you can find various solutions for it. The reason is that, when run as a script from the command line, python doesn't automatically recognize that it's a part of a package, and relative imports don't quite work as directories.

I suggest the following workflow:

  1. Install the package with: pip install -e .
  2. Add files and changes to the package.
  3. Import the package using absolute import and run the code.

As an example, create privacy_evaluator/attacks/membership_inference/test.py file with e.g. following contents:

def hello_world(): 
    print("Hello world")

Then you can do (from anywhere):

$ python3
>>> from privacy_evaluator.attacks.membership_inference.test import hello_world
>>> hello_world()
Hello world
travela commented 3 years ago

I see! Btw the same error arises when I run it in my VSC interactive window.

But you're absolutely right, with absolute imports it works and that was my concern with this issue. How do you want me to execute the code after switching to local imports then? Because so far I either do it in the terminal or the VSC

adrinjalali commented 3 years ago

Please note that generally you're not supposed to run the code written in a library directly in interactive mode. If you want to test something in the library, you can write a test, debug the test, and then follow the path and debug the code if that's what you're trying to do. This would also avoid your local import issues.

travela commented 3 years ago

Thanks a lot for your advice. It's my first time contributing to a library and I still need to learn the best practices. So I should run the test using pytest then in our case? Cheers!

adrinjalali commented 3 years ago

Yes. Here's a list of some of the ways I check my code when I'm coding for a library (you may want to put a version of this in your contributing guide if you think it may help others as well):

for both the above cases, I end up adding a ton of print statements to check for things at different steps

You could have links to documentation for each commonly used IDE for people to follow and get their environment setup.