Ticket #8355 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

Fix hsv_to_rgb to take all 3 arguments

Reported by: boothby Owned by: was
Priority: minor Milestone: sage-4.6.2
Component: graphics Keywords: beginner
Cc: kcrisman Work issues:
Report Upstream: N/A Reviewers: Willem Jan Palenstijn
Authors: Aly Deines, Robert Bradshaw Merged in: sage-4.6.2.alpha1
Dependencies: Stopgaps:

Description 

sage: hue(.5,.5,.5)
Traceback (click to the left of this block for traceback)
...
TypeError: can't multiply sequence by non-int of type 'float'

Attachments

8355-hue.patch Download (815 bytes) - added by robertwb 3 years ago.
8355-hue.2.patch Download (800 bytes) - added by aly.deines 2 years ago.
hue doctest which applies to sage-4.6.1.rc1
8355-hue.2.2.patch Download (800 bytes) - added by kcrisman 2 years ago.
Use only this patch

Change History

Changed 3 years ago by robertwb

comment:1 Changed 3 years ago by boothby

  • Status changed from new to needs_review
  • Milestone set to sage-4.3.5

comment:2 Changed 3 years ago by boothby

  • Status changed from needs_review to positive_review

Works for me.

comment:3 Changed 3 years ago by jhpalmieri

  • Status changed from positive_review to needs_work
  • Reviewers set to Tom Boothby
  • Authors set to Robert Bradshaw

This doesn't apply cleanly to me against 4.3.5. Please rebase.

comment:4 Changed 3 years ago by jhpalmieri

  • Milestone changed from sage-4.3.5 to sage-4.4

comment:5 Changed 3 years ago by jason

  • Keywords beginner added

comment:6 Changed 3 years ago by kcrisman

  • Cc kcrisman added

comment:7 Changed 2 years ago by ryan

  • Status changed from needs_work to needs_review

I think this has been fixed already. Works in sage-4.6.0

comment:8 Changed 2 years ago by kcrisman

Confirmed. But should we add a patch that confirms this, as in the previous patch? This is because we now use the Python version of this, afaict.

comment:9 Changed 2 years ago by aly.deines

I've created a patch that confirms 

sage: hue(.5,.5,.5)
(0.25, 0.5, 0.5)

in the doctest of hue and applies to sage-4.6.1.rc1.

Changed 2 years ago by aly.deines

hue doctest which applies to sage-4.6.1.rc1

comment:10 Changed 2 years ago by wjp

  • Status changed from needs_review to positive_review

Looks good to me.

comment:11 Changed 2 years ago by jdemeyer

  • Milestone changed from sage-4.6.1 to sage-4.6.2

comment:12 Changed 2 years ago by kcrisman

  • Reviewers changed from Tom Boothby to Willem Jan Palenstijn
  • Authors changed from Robert Bradshaw to Aly Deines

If these spellings of the author and reviewer aren't right, please correct them.

comment:13 Changed 2 years ago by aly.deines

  • Authors changed from Aly Deines to Aly Deines, Robert Bradshaw

comment:14 Changed 2 years ago by wjp

  • Reviewers changed from Willem Jan Palenstijn to Tom Boothby, Willem Jan Palenstijn

comment:15 Changed 2 years ago by kcrisman

Well, up to you guys. It just seemed to me that since there wasn't anything left to patch, maybe the older ones weren't relevant - but you can give credit where you feel it's due!

comment:16 Changed 2 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Please add the correct ticket number to the commit message :-)

Changed 2 years ago by kcrisman

Use only this patch

comment:17 Changed 2 years ago by kcrisman

  • Status changed from needs_work to needs_review

This should fix this. Apply only 8355-hue.2.2.patch

comment:18 Changed 2 years ago by kcrisman

  • Status changed from needs_review to positive_review

comment:19 Changed 2 years ago by boothby

  • Reviewers changed from Tom Boothby, Willem Jan Palenstijn to Willem Jan Palenstijn

I didn't have anything to do with the review of the patch going in... I think it's weird to get credit for this.

comment:20 Changed 2 years ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-4.6.2.alpha1
Note: See TracTickets for help on using tickets.