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

 
 

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

Submitter:  Markus Mützel <mmuetzel>
Submitted:  Thu 12 Jan 2017 07:47:00 PM UTC
   
 
Category:  Plotting Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Missed Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

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

 

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

Attach Files:
   
   
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
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mmuetzel (Submitted the item)
  • -email is unavailable- added by mmuetzel
  •  

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

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-01-31 pantxo StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2017-01-20 mmuetzel Attached File- Added bug50041_constraints_finite_type.patch, #39524
        StatusIn Progress Patch Submitted
    2017-01-17 jwe Item GroupIncorrect Result Missed Error or Warning
    2017-01-12 mmuetzel Attached File- Added wip_constraints_finite_type.patch, #39452
        Carbon-Copy- Added pantxo

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code