Skip to content

[MRG] Reraise when the exception is not a TransportableException#429

Merged
aabadie merged 1 commit intojoblib:masterfrom
lesteve:improve-non-transportable-exception-handling
Nov 24, 2016
Merged

[MRG] Reraise when the exception is not a TransportableException#429
aabadie merged 1 commit intojoblib:masterfrom
lesteve:improve-non-transportable-exception-handling

Conversation

@lesteve
Copy link
Copy Markdown
Member

@lesteve lesteve commented Nov 24, 2016

Usual caveat about catching an exception, raising the exception and losing some information in the stacktrace.

An example I bumped into when writing a custom parallel backend:

from joblib import Parallel, delayed
from joblib.parallel import ParallelBackendBase, parallel_backend


class Result(object):
    def __init__(self, result):
        self.result = result

    def get(self, timeout=None):
        1/0


class MyBackend(ParallelBackendBase):
    def apply_async(self, batch, callback):
        return Result(batch())

    def effective_n_jobs(self, n_jobs):
        return n_jobs


def func(i):
    return i

with parallel_backend(MyBackend(), n_jobs=2):
    Parallel(n_jobs=2)(delayed(func)(i) for i in range(10))

On Python 2.7 (on Python 3 the problem is less severe):
master:

Traceback (most recent call last):
  File "/tmp/test.py", line 25, in <module>
    Parallel(n_jobs=2)(delayed(func)(i) for i in range(10))
  File "/home/lesteve/dev/joblib/joblib/parallel.py", line 768, in __call__
    self.retrieve()
  File "/home/lesteve/dev/joblib/joblib/parallel.py", line 719, in retrieve
    raise exception
ZeroDivisionError: integer division or modulo by zero

this PR:

Traceback (most recent call last):
  File "/tmp/test.py", line 25, in <module>
    Parallel(n_jobs=2)(delayed(func)(i) for i in range(10))
  File "/home/lesteve/dev/joblib/joblib/parallel.py", line 771, in __call__
    self.retrieve()
  File "/home/lesteve/dev/joblib/joblib/parallel.py", line 682, in retrieve
    self._output.extend(job.get(timeout=self.timeout))
  File "/tmp/test.py", line 10, in get
    1/0
ZeroDivisionError: integer division or modulo by zero

On master you lose the line where the error actually happened. In my case it would have saved me a few hours of being really really confused.

aabadie
aabadie suggested changes Nov 24, 2016
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

LGTM (except a little typo)

joblib/parallel.py Outdated
self._aborting = True

if isinstance(exception, TransportableException):
# If the backends allows it, cancel or kill remaining running
Copy link
Copy Markdown
Contributor

@aabadie aabadie Nov 24, 2016

Choose a reason for hiding this comment

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

backend

Copy link
Copy Markdown
Member Author

@lesteve lesteve Nov 24, 2016

Choose a reason for hiding this comment

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

Not my typo ;-) (I just moved the code around) but I'll fix it anyway.

Copy link
Copy Markdown
Member Author

@lesteve lesteve Nov 24, 2016

Choose a reason for hiding this comment

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

Done

@lesteve lesteve force-pushed the improve-non-transportable-exception-handling branch from eec3b5d to 94f250d Compare November 24, 2016 15:29
aabadie
aabadie approved these changes Nov 24, 2016
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

LGTM +1

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Nov 24, 2016

Thanks @lesteve. Feel free to merge.

@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Nov 24, 2016

Thanks @lesteve. Feel free to merge.

Can you change your review to "Approved"?

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Nov 24, 2016

It's done already ;)

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Nov 24, 2016

Ah, I think you can't merge your own PR. So merginf, thanks !

@aabadie aabadie merged commit 8e777f5 into joblib:master Nov 24, 2016
yarikoptic added a commit to yarikoptic/joblib that referenced this pull request Dec 8, 2016
@yarikoptic
Merge commit '0.10.2-55-g660fe5d' into releases
f4720fc 
* commit '0.10.2-55-g660fe5d': (52 commits)
  MAINT simplify way to skip doctests
  Do not print frames with IPython internals in format_stack
  BF(TST): compare for version in pickled filename ignoring full path, and looking for _ as well (joblib#445)
  [MRG] Remove nose dependency completely. (joblib#441)
  MAINT add Python 3.5 in setup.py
  Remove some Python 2.6 specific code
  [MRG] Make test execution on CI possible through py.test command. (joblib#433)
  Remove more python 2.6 related code (joblib#440)
  [MRG] Use commit range for diff in flake8_diff.sh (joblib#439)
  MAINT remove support for python 2.6 (joblib#437)
  Improve doc about caching methods (joblib#432)
  Get the tests passing with py.test
  MAINT update flake8_diff.sh
  COSMIT move stdlib imports where they belong
  [MRG] Replace assert_* methods with "assert" keyword statements. (joblib#430)
  Reraise when the exception is not a TransportableException (joblib#429)
  Make sure LICENSE.txt is included in the wheel
  Add LICENSE.txt
  Update doc/paralle.rst
  [MRG+1] Remove all nose imports from test scripts. (joblib#422)
  ...
yarikoptic added a commit to yarikoptic/joblib that referenced this pull request Dec 8, 2016
@yarikoptic
Merge branch 'releases' into debian
5eeebd5 
* releases: (52 commits)
  MAINT simplify way to skip doctests
  Do not print frames with IPython internals in format_stack
  BF(TST): compare for version in pickled filename ignoring full path, and looking for _ as well (joblib#445)
  [MRG] Remove nose dependency completely. (joblib#441)
  MAINT add Python 3.5 in setup.py
  Remove some Python 2.6 specific code
  [MRG] Make test execution on CI possible through py.test command. (joblib#433)
  Remove more python 2.6 related code (joblib#440)
  [MRG] Use commit range for diff in flake8_diff.sh (joblib#439)
  MAINT remove support for python 2.6 (joblib#437)
  Improve doc about caching methods (joblib#432)
  Get the tests passing with py.test
  MAINT update flake8_diff.sh
  COSMIT move stdlib imports where they belong
  [MRG] Replace assert_* methods with "assert" keyword statements. (joblib#430)
  Reraise when the exception is not a TransportableException (joblib#429)
  Make sure LICENSE.txt is included in the wheel
  Add LICENSE.txt
  Update doc/paralle.rst
  [MRG+1] Remove all nose imports from test scripts. (joblib#422)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@aabadie aabadie aabadie approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lesteve @aabadie