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: MonitorCenter value are sometimes None #887

Merged
merged 1 commit into from Apr 12, 2015

Conversation

Projects
None yet
3 participants
@jeremygray
Member

jeremygray commented Apr 12, 2015

  • locale.str() needs a number, so ensure it gets one
BF: MonitorCenter value are sometimes None
- locale.str() needs a number, so ensure it gets one
@coveralls

This comment has been minimized.

coveralls commented Apr 12, 2015

Coverage Status

Coverage increased (+0.0%) to 50.56% when pulling 449e422 on jeremygray:master into 3db1023 on psychopy:master.

@peircej

This comment has been minimized.

peircej commented on 449e422 Apr 12, 2015

Does this mean that monitors then end up having the value zero for these attributes? That might mean that I need to check through places where it is used in case we just propogate an error to somewhere else. I'm thinking of scenarios where the code then says

if mon.getDistance() is not None:
doSomethingWithMonDistance()

Now we'll might need

if mon.getDistance() not in [None, 0]:
doSomethingWithMonDistance()

This comment has been minimized.

Owner

jeremygray replied Apr 12, 2015

I am unsure if those lines are only setting the value for display purposes, inside the relevant ctrls (in which case it should be no problem? or at least not the problem you raise), or if those changes are later propagated back to self.currentCalib['distance'] (in which case it might be an issue).

mon.getDistance() just returns self.currentCalib['distance'].

This comment has been minimized.

peircej replied Apr 13, 2015

peircej added a commit that referenced this pull request Apr 12, 2015

Merge pull request #887 from jeremygray/master
BF: MonitorCenter value are sometimes None

@peircej peircej merged commit 46e1cb6 into psychopy:master Apr 12, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.0%) to 50.56%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment