patchGNU Octave - Patches: patch #8060, [image package new function]...

 
 

patch #8060: [image package new function] whitepoint.m

Submitted by:  Ming Liu <liuming198904>
Submitted on:  Sat 25 May 2013 06:26:50 AM UTC  
 
Category: NonePriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: Carnë Draug <carandraug>Open/Closed: Open

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Tue 28 May 2013 09:37:17 PM UTC, comment #7:

As Patrick already mentioned, Matlab does in some cases use more precision. You can also see it if you set `format long`. Apparently, it only does it for some of the standards but I think it's better if we can be consistent.

Also, please remember that you should not look at the source of Matlab code when implementing these functions for Octave. You also shouldn't be using Matlab at all, even for testing. Follow their documentation, and if really required, ask someone to run a test for you and give you the results. Users in ##matlab on the freenode IRC servers are quite helpful.

Carnë Draug <carandraug>
Project MemberIn charge of this item.
Tue 28 May 2013 04:57:52 PM UTC, comment #6:

Try this:
sprintf('%.8f\t', whitepoint('d50'))

This will return more than 4 significant digits.

I'd suggest you increase the precision. However, calculating the values using the formula Carnë mentioned is not trivial.

I am reviewing my copy of CIE 15:2004, which I think will have a good suggestion for calculating the whitepoints. I'll post more here later.

Patrick Noffke <pnoffke>
Sun 26 May 2013 02:04:27 PM UTC, comment #5:

A simple question:
I just tested the same thing in MATLAB, it has 4 digits precision. Do we still need to increase the precision?
Ming Liu

Ming Liu <liuming198904>
Sun 26 May 2013 01:35:50 PM UTC, comment #4:

Of course you can. Looking forward to the fixed version.

Carnë Draug <carandraug>
Project MemberIn charge of this item.
Sun 26 May 2013 09:00:35 AM UTC, comment #3:

Thank you for all the replies!!
Hi Carnë, may I use your code in my re-submission?

Ming Liu <liuming198904>
Sat 25 May 2013 04:33:49 PM UTC, comment #2:

Hi!

Aside the case insensitivity issue, which can be solved with using

there's a few other issues:

1) no documentation. Could you please write documentation in TexInfo. See other functions in Octave for an example. It's pretty simple.
2) your function only has 4 digit precision, but there's an actual formula which can give a much correct value. See http://en.wikipedia.org/wiki/Standard_illuminant
3) no input check. For this case, the function should start with:

Note on how we are also setting the default value right at the start.

In addition, other things that are just the guidelines for contributions

4) the main function block should also be indented
5) use double quote for strings unless (there's a really good reason otherwise)
6) error message must mention the function name, the variable name (in caps) as it's named in the documentation (you don't have documentation yet so I'm calling it standard), and the wrong value that was given. So it should be something like:

Note that if you made input check as I mentioned above, you'll know it's a string, so you can use %s.

7) specify what block is ending with endfunction, endswitch, etc...

Carnë Draug <carandraug>
Project MemberIn charge of this item.
Sat 25 May 2013 03:58:10 PM UTC, comment #1:

This should be case-insensitive (i.e. whitepoint('D50') should work).

Other than that, it looks good.

Patrick Noffke <pnoffke>
Sat 25 May 2013 06:26:50 AM UTC, original submission:

XYZ color values of standard illuminants

usage:
xyz = whitepoint(string)
string is one of the following:
a, c, icc, d50, d55, d65'

Ming Liu <liuming198904>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #28168:  Ming_Liu_whitepoint.patch added by liuming198904 (1kB - application/octet-stream)
file #28169:  whitepoint.m added by liuming198904 (1kB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by pnoffke (Posted a comment)
  • -unavailable- added by liuming198904 (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 28 May 2013 06:43:52 PM UTCjweCarbon-CopyRemoved -unavailable-=>-
    Sat 25 May 2013 04:33:49 PM UTCcarandraugAssigned toNone=>carandraug
    Sat 25 May 2013 06:26:51 AM UTCliuming198904Carbon-Copy-=>Added -unavailable-
    Sat 25 May 2013 06:26:50 AM UTCliuming198904Attached File-=>Added Ming_Liu_whitepoint.patch, #28168
      Attached File-=>Added whitepoint.m, #28169

    Back to the top


    Powered by Savane 3.1-cleanup