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

Make local modules importable when running verdi run #3700

Merged
merged 1 commit into from
Dec 24, 2019

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Dec 20, 2019

Fixes #3624

Running a python script through verdi run from its local directory
that imports from a module in the same directory would yield an
ModuleNotFoundError. The problem is because the current working
directory was not being passed in the sys.path of the exec'ed file.

Copy link
Member

@giovannipizzi giovannipizzi left a comment

Choose a reason for hiding this comment

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

I don't think this works, for 2 reasons:

  1. sys.path.append appends in place, so it returns None
  2. I don't think adding a key sys.path to the global dict does the trick

The right approach would be to add

import os                                                                  
sys.path.append(os.path.abspath(os.curdir))   

in update_environment, ideally storing a copy into a variable _path before appending, and then resetting after the yield

@sphuber
Copy link
Contributor Author

sphuber commented Dec 23, 2019

@giovannipizzi , thanks you are right. It just "worked" by accident. I tested it by doing the following. I created a folder testing and within a file module.py:

def test():
    print('test')

and a file script.py:

from module import test

if __name__ == '__main__':
    test()

Calling verdi run script.py from within the testing folder without the fix would raise:

(aiida_dev) sph@bastion:~/code/aiida/env/dev/aiida-core/testing$ verdi run script.py 
Traceback (most recent call last):
  File "/home/sph/.virtualenvs/aiida_dev/bin/verdi", line 10, in <module>
    sys.exit(verdi())
  File "/home/sph/.virtualenvs/aiida_dev/lib/python3.6/site-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/home/sph/.virtualenvs/aiida_dev/lib/python3.6/site-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/home/sph/.virtualenvs/aiida_dev/lib/python3.6/site-packages/click/core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/sph/.virtualenvs/aiida_dev/lib/python3.6/site-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/sph/.virtualenvs/aiida_dev/lib/python3.6/site-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/home/sph/code/aiida/env/dev/aiida-core/aiida/cmdline/utils/decorators.py", line 65, in wrapper
    return wrapped(*args, **kwargs)
  File "/home/sph/code/aiida/env/dev/aiida-core/aiida/cmdline/commands/cmd_run.py", line 114, in run
    exec(compile(handle.read(), scriptname, 'exec', dont_inherit=True), globals_dict)  # yapf: disable # pylint: disable=exec-used
  File "script.py", line 2, in <module>
    from module import test
ModuleNotFoundError: No module named 'module'

With my original "fix" the problem was solved but not for the reasons I thought. Simply the act of calling sys.path.append was enough because apparently the sys is shared/kept when the exec is invoked. The no-op assignment to the globals dict was then exactly that, a no-op. I have corrected the code to move it to the context manager as suggested.

Copy link
Member

@giovannipizzi giovannipizzi left a comment

Choose a reason for hiding this comment

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

Thanks! One more thing, I think this is not enough to restore the path, since _path is just a reference and not a copy and also when restored will still have the appended content. I think it should be defined as a copy

Running a python script through `verdi run` from its local directory
that imports from a module in the same directory would yield an
`ModuleNotFoundError`. The problem is because the current working
directory was not being passed in the `sys.path` of the exec'ed file.
@sphuber
Copy link
Contributor Author

sphuber commented Dec 24, 2019

really sorry @giovannipizzi for the sloppyness; my head must be already at christmas dinner :)

@giovannipizzi giovannipizzi merged commit a6c4704 into aiidateam:develop Dec 24, 2019
@giovannipizzi giovannipizzi deleted the fix_3624_verdi_run_cwd branch December 24, 2019 15:16
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.

ModuleNotFoundError when using "verdi run" with script which has local import statement
2 participants