Skip to content
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

BF: iohub python 3 compatibility #1775

Merged
merged 3 commits into from Mar 21, 2018

Conversation

ariekahn
Copy link
Contributor

A few minor issues cropping up with iohub on Python 3:

  1. trailing commas were causing a few of the keycodes to be represented as tuples, which was ignored in python 2 but in 3 gives:
TypeError: '>' not supported between instances of 'tuple' and 'int'
  1. darwinkey needed to be a relative import

  2. locals() returns a view in python 3, which means we can't change it when iterating over it. Explicitly creating a copy should preserve the old behavior.

@peircej
Copy link
Member

peircej commented Mar 21, 2018

Great. Well done. I think we're going to find quite a few of these issues, especially in iohub, so it's good to have someone jumping in there on it. I'll be trying to push out a few quick bug-fix releases.

Oh, one thing though, could you annotate your commits in future with BF: for bug-fixes FF: for fixes to not-yet-released features and ENH for new features/enhancements. The labels in the oull request doesn't matter but the commit message is important. When I create a bug fix release I typically do that in a separate git branch and only commits tagged with BF: get included

@coveralls
Copy link

coveralls commented Mar 21, 2018

Coverage Status

Coverage increased (+0.006%) to 51.767% when pulling 7cd627c on ariekahn:FIX/iohub-py3-issues into 0133fbc on psychopy:master.

@codecov-io
Copy link

Codecov Report

Merging #1775 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1775      +/-   ##
==========================================
+ Coverage   47.08%   47.09%   +0.01%     
==========================================
  Files         218      218              
  Lines       33284    33284              
  Branches     5556     5556              
==========================================
+ Hits        15671    15675       +4     
+ Misses      16055    16053       -2     
+ Partials     1558     1556       -2
Impacted Files Coverage Δ
psychopy/data/base.py 71.87% <0%> (-0.9%) ⬇️
psychopy/tools/wizard.py 74.45% <0%> (+0.65%) ⬆️
psychopy/visual/textbox/fontmanager.py 69.96% <0%> (+1.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0133fbc...7cd627c. Read the comment docs.

@peircej peircej merged commit 7fad95a into psychopy:master Mar 21, 2018
@ariekahn
Copy link
Contributor Author

My bad! Will annotate any commits themselves in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants