Skip to content

BF: Reset marker color after a response#3140

Merged
peircej merged 1 commit intopsychopy:masterfrom
pn2200:reset-marker-color
Sep 18, 2020
Merged

BF: Reset marker color after a response#3140
peircej merged 1 commit intopsychopy:masterfrom
pn2200:reset-marker-color

Conversation

@pn2200
Copy link
Contributor

@pn2200 pn2200 commented Sep 16, 2020

Should resolve #3139 .

Store the original marker color, and restore that color during reset,
so the marker is reset for the next trial.
@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2020

Codecov Report

Merging #3140 into master will increase coverage by 0.00%.
The diff coverage is 83.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3140   +/-   ##
=======================================
  Coverage   43.52%   43.52%           
=======================================
  Files         260      260           
  Lines       49855    49858    +3     
  Branches     8503     8503           
=======================================
+ Hits        21699    21702    +3     
  Misses      25889    25889           
  Partials     2267     2267           
Impacted Files Coverage Δ
psychopy/visual/ratingscale.py 83.71% <83.33%> (+0.07%) ⬆️

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 5451ea4...7d7900f. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 47.735% when pulling 7d7900f on pn2200:reset-marker-color into 9885bc2 on psychopy:master.

@peircej peircej requested a review from TEParsons September 17, 2020 09:29
@peircej
Copy link
Member

peircej commented Sep 18, 2020

Thanks Paul. Couple of comments:

  • Are both the following lines needed? Most attributes in PsychoPy include setter code so you don't need the second step (although Rating Scale is old and maybe never got retrofitted with that)
        self.markerColor = self.markerColorOriginal
        self._setMarkerColor(self.markerColor)
  • I'd generally recommend moving to Slider instead of RatingScale. Rating is big and complex. Slider handles the same features and more (including styling optoins like radio buttons, vertical orientation...) and actually renders much faster too. It's also the only option that ports to JavaScript. That doesn't mean we shouldn't fix bugs in Rating but I'd say it's essentially deprecated for the long-term

@pn2200
Copy link
Contributor Author

pn2200 commented Sep 18, 2020

  • I think both lines are needed. self.markerColor is only an attribute on the RatingScale. Setting it doesn't modify the actual marker object. The second line tries to modify the marker object by setting the color or fillColor attributes. I think it tries to set the color or fillColor attributes to allow for custom marker objects that may only implement one or the other.
  • Fair point about RatingScale being essentially deprecated. I will keep that in mind when developing new tasks.

@peircej peircej merged commit 66ac190 into psychopy:master Sep 18, 2020
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.

RatingScale marker turns grey if the scale times out and does not refresh on next trial

4 participants