Skip to content

should dv.sync_import print failed imports ? #1208

Closed
Carreau opened this Issue Dec 25, 2011 · 3 comments

2 participants

@Carreau
IPython member
Carreau commented Dec 25, 2011

Hi,
I'm trying parallel magic, I just feel that sync_import should at least warn user if some import have failed/are not done, or at least state in the docstring that if import failed/are not done it stays silent.

Docstring don't mention local keyword, (should it ? even if no yet implemented ? )

@minrk
IPython member
minrk commented Dec 26, 2011

Yes and yes. The comments in the code clearly state:

            if not local:
                # ignore import errors if not doing local imports
                pass

But there needs to be an else: raise after that to actually raise the error.

@minrk minrk added a commit that closed this issue Dec 26, 2011
@minrk minrk DirectView.sync_imports fixes
ImportErrors are properly raised when no such package exists

describe `local` kwarg's purpose, indicating that `local=False`
is not implemented.

closes #1208
bf7d20d
@minrk minrk closed this in bf7d20d Dec 26, 2011
@minrk
IPython member
minrk commented Dec 26, 2011

should be fixed

@Carreau
IPython member
Carreau commented Dec 26, 2011

Yes, it works.
just one other detail, maybe due to @contexmanager
When ? or ?? docstring state :
Definition: dv.sync_imports(*args, **kwds) which don't cite the definition of the non decorated function. Same with temp_flags but there is no parameters so not really important.

@yarikoptic yarikoptic pushed a commit to yarikoptic/ipython that referenced this issue May 2, 2014
@minrk minrk DirectView.sync_imports fixes
ImportErrors are properly raised when no such package exists

describe `local` kwarg's purpose, indicating that `local=False`
is not implemented.

closes #1208
2d2cd9e
@mattvonrocketstein mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
@minrk minrk DirectView.sync_imports fixes
ImportErrors are properly raised when no such package exists

describe `local` kwarg's purpose, indicating that `local=False`
is not implemented.

closes #1208
23f990d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.