bugGNU Octave - Bugs: bug #50041, Setting non-valid values for...

 
 

bug #50041: Setting non-valid values for graphics properties should throw an error

Submitted by:  Markus Mützel <mmuetzel>
Submitted on:  Thu 12 Jan 2017 07:47:00 PM UTC  
 
Category: PlottingSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Missed Error or Warning
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: Any

Add a New Comment(Rich 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 31 Jan 2017 10:13:25 PM UTC, comment #6:

Markus, the patch works for me and I pushed it without modification here:

http://hg.savannah.gnu.org/hgweb/octave/rev/9e54152e382b

Closing report as fixed.

Pantxo Diribarne <pantxo>
Project Member
Fri 20 Jan 2017 08:46:38 PM UTC, comment #5:

Attached is a cleaned up patch that adds support for finite_type constraints and extends the support of range constraints to row_vector and array properties.

Thank you for reviewing.

(file #39524)

Markus Mützel <mmuetzel>
Project Member
Thu 19 Jan 2017 12:17:38 AM UTC, comment #4:

Thank you, Pantxo, for reviewing.

I was trying to add the new constraints to all properties that would need them. But checking all double, row_vector and array properties in Matlab for NaN, Inf and negative values is quite tedious.

Additionally, some properties have quite unique constraints. E.g. the "*position" properties of figure are allowed to have negative values in the first two elements (x and y position). But the last two elements (width and height) must be positive. The generic constraints that we have cannot perform this kind of check. Checking this in the update_NAME function is too late because the values are already set and the old values are overwritten. Even if we still can throw an error there, we cannot restore the previous (working) state. Maybe there should be an optional check_NAME function that could be called before the value is set?
But this can probably wait for a later patch.

Anyway, I will clean up what I have so far and post the updated patch as soon as it is ready and tested.

Markus Mützel <mmuetzel>
Project Member
Wed 18 Jan 2017 09:46:56 PM UTC, comment #3:

Markus, the approach looks good to me to. Can you remove the "row_vector_property" part of the patch write the changelog? Or do you intend to make other changes?

Pantxo Diribarne <pantxo>
Project Member
Fri 13 Jan 2017 04:03:35 PM UTC, comment #2:

jwe: I posted a changeset for the array -> row_vector property type in bug #50048.

Markus Mützel <mmuetzel>
Project Member
Thu 12 Jan 2017 08:20:41 PM UTC, comment #1:

Could you make the array -> row_vector property type as a separate changeset?

I might be missing something, but additional constraints seems like the right thing to do to me.

John W. Eaton <jwe>
Project Administrator
Thu 12 Jan 2017 07:47:00 PM UTC, original submission:

Setting non-valid values for some properties leads to un-expected results (e.g. bug #49755: axis equal clears plot). These should instead throw an error (as they do in Matlab).

Attached is a work in progress patch that adds new constraints (finite_type) for array_properties, row_vector_properties and double_properties. It also expands the support of min-max-constraints that we already have for double_properties to array_properties and row_vector_properties.
I added these constraints to some of the properties to which they apply. However, I would like to know whether this is the correct approach before I continue.

I am adding Pantxo to the CC list since this is very similar to what he did in task #11311. Pantxo, could you please comment when you have the time?

I also changed "plotboxaspectratio" and "dataaspectratio" from array_properties to row_vector_properties since this is how they are handled in Matlab. Should this change be made in a separate bug report?

Markus Mützel <mmuetzel>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #39524:  bug50041_constraints_finite_type.patch added by mmuetzel (11KiB - application/octet-stream)
file #39452:  wip_constraints_finite_type.patch added by mmuetzel (11KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by pantxo (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by mmuetzel (Submitted the item)
  • -unavailable- added by mmuetzel
  •  

    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 project members can vote.

     

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

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 31 Jan 2017 10:13:25 PM UTCpantxoStatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Fri 20 Jan 2017 08:46:38 PM UTCmmuetzelAttached File-=>Added bug50041_constraints_finite_type.patch, #39524
      StatusIn Progress=>Patch Submitted
    Tue 17 Jan 2017 03:17:14 PM UTCjweItem GroupIncorrect Result=>Missed Error or Warning
    Thu 12 Jan 2017 07:47:00 PM UTCmmuetzelAttached File-=>Added wip_constraints_finite_type.patch, #39452
      Carbon-Copy-=>Added pantxo

    Back to the top


    Powered by Savane 3.1-cleanup1