-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update supertask interface and many related classes. #17
Conversation
f7f907c
to
a4c71d3
Compare
62987ab
to
8472bd5
Compare
Still WIP, many updates to parser, SuperTask, SuperTaskConfig, unit test and examples.
8472bd5
to
0440bd0
Compare
Re-enable unit tests for graph builder and supertask.
@@ -307,7 +303,7 @@ def runPipeline(self, graph, butler, args): | |||
mapFunc = _MPMap(numProc, timeout) | |||
else: | |||
# map in Py3 returns iterable and we want a complete result | |||
mapFunc = lambda func, iterable: list(map(func, iterable)) | |||
mapFunc = lambda func, iterable: list(map(func, iterable)) # noqa: E731 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we really should have a def mapFunc
here rather than something which looks like python word salad (and we don't need the python2 code).
@@ -346,13 +342,12 @@ def _executeSuperTask(self, target): | |||
return task.runQuantum(quantum, butler) | |||
except Exception: | |||
raise # No worries | |||
except: | |||
except: # noqa: E722 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. This should be BaseException
if you are really wanting to intercept Ctrl-C and the like. I'm not sure that's a good idea though, especially since the BaseException
converted to an Exception
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was copied from CmdLineTask
, I think the purpose of this code is to catch non-BaseException exceptions (like raise None
) and convert it into standard Exception. I think this is done because multiprocessing
fails badly if it sees non-Exception exceptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might be how Python 2 worked but if I do:
try:
print("About to raise")
raise None
except RuntimeError:
print("Caught RuntimeError")
except Exception:
print("Caught Exception")
except BaseException:
print("Caught BaseException")
then I get:
About to raise
Caught Exception
So I think this should be catching BaseException
if you want to trap everything including Ctrl-C.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I get the same answer with Python 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I did some experimenting, indeed behavior for raise None
is (almost) identical between 2 and 3. The difference is for old-style classes used as exception in py2, they are not caught by BaseException or Exception. And you are right that it does not matter for py3, so I should drop that.
6189225
to
83f2d5c
Compare
Still WIP, many updates to parser, SuperTask, SuperTaskConfig, unit test
and examples.