patchGNU Octave - Patches: patch #8060, [octave forge] (image): new...

 
 

patch #8060: [octave forge] (image): new function whitepoint.m

Submitter:  Ming Liu <liuming198904>
Submitted:  Sat 25 May 2013 06:26:50 AM UTC
   
 
Category:  Forge : new function Priority:  5 - Normal
Status:  None Privacy:  Public
Assigned to:  carandraug Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

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>
Group Member
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>
Group Member
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


  switch (tolower (string))


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:


function xyz = whitepoint (standard = "icc")
  if (nargin > 1)
    print_usage ();
  elseif (! ischar (standard))
    error ("whitepoint: STANDARD must be a string.");
  endif


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:


  error ("whitepoint: unknown STANDARD `%s'.", standard);


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>
Group Member
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 Files:
   
   
Comment:
   

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

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-01-18 siko1056 CategoryNone Forge : new function
        Summary[image package new function] whitepoint.m [octave forge] (image): new function whitepoint.m
    2013-05-28 jwe Carbon-CopyRemoved -email is unavailable- -
    2013-05-25 carandraug Assigned toNone carandraug
    2013-05-25 liuming198904 Carbon-Copy- Added -email is unavailable-
    2013-05-25 liuming198904 Attached File- Added Ming_Liu_whitepoint.patch, #28168
        Attached File- Added whitepoint.m, #28169

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code