octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Patch for datetick.m to improve ML compatibiity


From: John W. Eaton
Subject: Patch for datetick.m to improve ML compatibiity
Date: Wed, 20 Jan 2010 14:51:45 -0500

On 20-Jan-2010, Philip Nienhuis wrote:

| While Matlab's datetick.m can handle plots without any actual argument 
| supplied (and then defaults to "x-axis" and an optimal tick format for 
| the tick range found), octave's datetick.m currently insists on having 
| at least argument "FORM" specified.
| 
| A bit surprising as:
| 
| (1) After the test for presence of argument FORM, in subfunction 
| __datetick__, that very argument is first set to [] (empty) around line 80;
| 
| (2) A bit further down the logic to determine optimal tick formats has 
| been implemented already - and seems to be used too, still further down.
| 
| A simple patch to comment out the lines hampering ML compatibility is 
| attached.
| I tested it and it seems to work OK.
| Hopefully the diff is in an acceptable format.

It's not.  A simple diff can't be applied automatically by patch.  You
need to send a context diff.  If you are using the Mercurial sources,
then you should send a Mercurial changeset instead of a plain context
diff.

| In the patch I've also added a line (currently commented out) to set the 
| month format to "19" (i.e. dd/mm) rather than "6" (mm/dd) for in case 
| datetick.m determines that the ticks should be represented by month 
| values; European users might prefer this.
| Obviously it might be better to interrogate the system's language 
| settings and then decide on this particular format but I wouldn't know 
| how to do that - let alone how to do that in a platform-independent way :-(

| 38,40c38,40
| <   if (nargin < 1)
| <     print_usage ();
| <   else
| ---
| > #  if (nargin < 1)
| > #    print_usage ();
| > #  else
| 48c48
| <   endif
| ---
| > #  endif

If you are changing this function so that it can accept any number of
arguments, then you should remove the code and reindent, instead of
commenting out some section.

| 107,113c107,113
| <   if (isnumeric (form))
| <     if (! isscalar (form) || floor (form) != form || form < 0)
| <       error ("datetick: expecting form argument to be a positive integer");
| <     endif
| <   elseif (! ischar (form) && !isempty (form))
| <     error ("datetick: expecting valid date format string");
| <   endif
| ---
| > #  if (isnumeric (form))
| > #    if (! isscalar (form) || floor (form) != form || form < 0)
| > #      error ("datetick: expecting form argument to be a positive integer");
| > #    endif
| > #  elseif (! ischar (form) && !isempty (form))
| > #    error ("datetick: expecting valid date format string");
| > #  endif

Shouldn't we still have checks if FORM is provided?  I guess this
should be instead inside an 

 if (! isempty (form)) ... endif

block.

| 196c196,197
| <       form = 6;
| ---
| >       form = 6;                     # for US users
| >     # form = 19;            # for European users  

Instead of adding a line that is commented out, I'd prefer to see

  ## FIXME -- FORM should be 19 for European users who use dd/mm
  ## instead of mm/dd.  How can that be determined automatically?

I checked in the following change to fix these problems:

  http://hg.savannah.gnu.org/hgweb/octave/rev/c2f1cdb59821

Now datetick succeeds with no arguments, but I don't think it is
generating the correct tick labels.  I'm seeing

 00:00  05:00  10:00  15:00  20:00  01:00

What should the labels be when the x-axis range is [0, 1], and what
change in datetick is needed?

jwe


reply via email to

[Prev in Thread] Current Thread [Next in Thread]