-
Notifications
You must be signed in to change notification settings - Fork 536
Enh/graphsubmission #456
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
Enh/graphsubmission #456
Conversation
Tiny bugfix for the CondorDAGMan plugin (plus a merged master branch, sorry)
from ordereddict import OrderedDict | ||
import sys | ||
if not sys.version_info < (2, 7): | ||
from ordereddict import OrderedDict |
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.
Where is it used? What happens for python 2.6?
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.
The logic here is backwards, the ordereddict module is only needed in python 2.6 (or less) and in python 2.7 (or 3.x) we need to import OrderedDict from collections.
Not sure when the OrderedDict is used though.
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.
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.
since OrderedDict is not in 2.6, i presume this is preferred.
if not sys.version_info < (2, 7):
from collections import OrderedDict
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.
@satra I think you had it right the first time, but the ordereddict module also needs to be added to the dependencies in setup.py if the python version is less than 2.6
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 have a branch with what I am proposing here: moloney@ff10dc7
This also addresses an issue where the original exception gets lost (and the crashdump does not get written) if an exception occurs within the try block but before importing savepkl. Let me know if you want me to open a separate pull request.
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.
two thoughts.
- the only reason we have ordereddict is because configparser uses it in 2.7. therefore in 2.6 we don't have this awkwardness. that's why i suggested the change above. what do you think?
- regarding your changes could you please send a pull-request to my branch. i can include those in.
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, makes sense that we don't need ordereddict for python < 2.7.
I opened a PR against your repo Satra: satra#14
Adding Notebook tutorials to quickstart
This reverts commit 36166c4.
Fix/removetraitschanges
Added reorient2std interface
Also makes writing of crashdumps more reliable.
Python27 fix
michael's condor changes