RF: Python 2/3 compatibility #525
Conversation
work in progress
BF - making code py2/3 compatible
making code py2/py3 compatible
2 similar comments
Codecov Report
@@ Coverage Diff @@
## master #525 +/- ##
=========================================
Coverage ? 23.11%
=========================================
Files ? 306
Lines ? 36103
Branches ? 0
=========================================
Hits ? 8344
Misses ? 27759
Partials ? 0
Continue to review full report at Codecov.
|
conversion to py2/3
|
Thanks! |
mvpa2/base/config.py
Outdated
| @@ -225,7 +225,7 @@ def get_as_dtype(self, section, option, dtype, default=None): | |||
| return default | |||
| try: | |||
| return SafeConfigParser._get(self, section, dtype, option) | |||
| except ValueError, e: | |||
| except ValueError as e: | |||
| # provide somewhat descriptive error | |||
| raise ValueError, \ | |||
There was a problem hiding this comment.
While at it, this statement could also be fixed right away to raise ValueError("....")
making constraints compatible.
I'm not completely certain about this one, given the printing of separate lines. Please advise as necessary!
There was a problem hiding this comment.
left a few comments around - please clarify on import of builtins and bring back docstrings empty lines ;) otherwise -- lovely. We would need to wait for travis to start working properly again -- seems to be not picking up for testing across many projects ATM
mvpa2/base/config.py
Outdated
| raise ValueError, \ | ||
| "Failed to obtain value from configuration for %s.%s. " \ | ||
| "Original exception was: %s" % (section, option, e) | ||
| raise ValueError("Failed to obtain value from configuration for %s.%s." \ |
There was a problem hiding this comment.
Just FYI: no trailing \ would longer be necessary here since now both strings are within ()
| from builtins import map | ||
| from builtins import str | ||
| from past.builtins import basestring | ||
| from builtins import object |
There was a problem hiding this comment.
why explicit imports from builtins are necessary? shouldn't it all work just fine without?
There was a problem hiding this comment.
ha ha -- I can answer my own question which came because I am (was) primarily six user, and not of the future. So builtins (and the past) is the compatibility layer from the future package. Thanks ;)
mvpa2/base/constraints.py
Outdated
| class Constraint(object): | ||
| """Base class for input value conversion/validation. | ||
|
|
There was a problem hiding this comment.
we are following numpy docstrings convention. So empty line here is (was ;)) on purpose. See https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt#id6
| An arbitrary number of constraints can be given. They are evaluated in the | ||
| order in which they were specified. The return value of each constraint is | ||
| passed an input into the next. The return value of the last constraint | ||
| is the global return value. No intermediate exceptions are caught. | ||
|
|
There was a problem hiding this comment.
here also some intensional blank lines should come back
There was a problem hiding this comment.
sorry, professor. Futurize automatically plays around with some of the spacing. I'll work on going through and making sure the spacing stays the same.
There was a problem hiding this comment.
btw -- the other spaces (two empty lines between classes etc) looked good to me. Just use "pycharm" IDE and it would also do the same suggestion(s)
per suggested changes
per suggestion
This is for ease of editing on my computer and to ensure that these changes can be pushed to git more easily.
per prof halchenko
planning to futurize files ad-hoc as error messages come up while running tests to ensure py3 compatibility of the code. suggestions/contributions welcome
… more documentation needed old py2 codes included in files with the same name with the ending ".bak" Please enter the commit message for your changes. Lines starting
…ger used in RL
=== Do not change lines below ===
{
"chain": [],
"cmd": "bash -c 'sed -i -e \"/if __name__ == .__main__..*no cover/,+5d\" {inputs}'",
"exit": 0,
"extra_inputs": [],
"inputs": [
"mvpa2/*/test*py",
"mvpa2/*/*/test*py",
"mvpa2/*/*/*/test*py"
],
"outputs": [
"mvpa2/*/test*py",
"mvpa2/*/*/test*py",
"mvpa2/*/*/*/test*py"
],
"pwd": "."
}
^^^ Do not change lines above ^^^
…er used in RL
=== Do not change lines below ===
{
"chain": [],
"cmd": "bash -c 'sed -i -e \"/def suite().*no cover/,+2d\" {inputs}'",
"exit": 0,
"extra_inputs": [],
"inputs": [
"mvpa2/*/test*py",
"mvpa2/*/*/test*py",
"mvpa2/*/*/*/test*py"
],
"outputs": [
"mvpa2/*/test*py",
"mvpa2/*/*/test*py",
"mvpa2/*/*/*/test*py"
],
"pwd": "."
}
^^^ Do not change lines above ^^^
…n random order of parameters in the test
…cebacks not even sure if that traceback information embedding would help us much, and ATM I saw no easy way to do that reraising for both 2 and 3 so just disabled that
… versions encoded so make explicit in the test
…ttribute PY3 I believe just ignoress it
We did not handle them at all before so saving many types in PY3 would have not worked
…ation warning about numpy.testing.decorators
making code py2/py3 compatible per yarikoptic's instructions. please lmk if I did anything procedurally incorrectly.
TODOs (@yarikoptic's edits):
converted.
While at it, I will try to move tests around (possibly screwing up some manual collection of tests) so we could adhere to the same "locality" setup as we do in other projects
base/done in the sense that you can run tests on it, but not completely:test_hdf.pyfor now resides under maintests/since pulls in too many other submodulesmaster) for py2. In many places we just use.items(),.keys()etc -- judging from timings of the run on travis -- there is sever hit ATM, but may be it is solely because we have more external dependencies installed for py2 than py3