Mon 06 Jun 2016 05:58:13 PM UTC, comment #10:
I pushed the patch as is:
http://hg.savannah.gnu.org/hgweb/octave/rev/d9fa4558d2ed
Thanks
|
Fri 03 Jun 2016 10:03:22 AM UTC, comment #9:
The latest patch is actually what I wanted to propose in comment #6. I'll test it and eventually push it in the beginning of next week. Thanks Markus for following up.
|
Fri 03 Jun 2016 09:36:27 AM UTC, comment #8:
Yet another patch where the change of warning states is moved inside base_graphics_object::reset_default_properties.
(file #37371)
|
Fri 03 Jun 2016 09:16:38 AM UTC, comment #7:
I tried a similar approach by changing the warning states in "reset" and it works, too (see attached patch). However, I remembered some discussion about scopes ("local", "global", ...) of warning and error states. I did not find anything related to it in error.h and did not want to cause yet another side effect by removing those scopes while changing the warning state.
So I changed over to taking the same approach as taken for some of the properties of the figure go.
But maybe you are right and there might be some properties in the future that would be deprecated but should be re-set anyway...
Putting this inside the loop should probably not be an issue. And that would prevent that effect from re-surfacing wherever reset_default_properties might be called.
I'll post a patch taking your approach from comment #6 as well and someone that has better insight than me can decide which should be taken...
(file #37370)
|
Fri 03 Jun 2016 08:39:11 AM UTC, comment #6:
If we want to extend the use of this warning (e.g. in root figure properties) a simpler approach could be to disable the "deprecated-property" warning locally in base_properties::reset_default_properties. Did you try this?
|
Thu 02 Jun 2016 06:17:32 PM UTC, comment #5:
That patch had an unexpected side effect: When "reset" is called on a patch or surface, the warning that "normalmode" was deprecated is issued. That warning could possibly confuse unaware users. (Or it might throw an error for "reset" if the warning state of "Octave:deprecated-property" was set to "error".)
Steps to reproduce:
Attached please find a patch that skips "normalmode" for patch and surface objects when reset.
As "normalmode" is always shadowed from "vertexnormalmode", there is no need to reset it anyway.
(file #37361)
|
Thu 28 Apr 2016 09:18:38 PM UTC, comment #4:
Thanks Markus. I pushed your patch here:
http://hg.savannah.gnu.org/hgweb/octave/rev/a0ebc922fd52
Closing report.
|
Thu 28 Apr 2016 11:15:54 AM UTC, comment #3:
Thank you, Pantxo, for reviewing the patch and for your feedback.
Attached please find an updated patch which adds a description for Octave:deprecated-property to warning_ids.m.
There was no item for Octave:deprecated-function either so I added one for that warning id as well. I hope this is OK even though this is a little off scope of this bug.
(file #37010)
|
Thu 28 Apr 2016 06:08:33 AM UTC, comment #2:
Confirmed. The patch looks good and I think the addition of this new warning is a good thing as well.
Could you add a description in warning_ids.m so that it is mentioned in the manual?
|
Wed 27 Apr 2016 12:00:11 PM UTC, comment #1:
That fix was faster than expected.
The id of the warnings is "Octave:deprecated-property". I did not find any other references to it in the source code but felt that "Octave:deprecated-function" did not quite fit here...
Is this the right approach to face out graphics properties?
(file #37003)
|
Wed 27 Apr 2016 08:33:17 AM UTC, original submission:
In recent versions of Matlab, "FaceNormals" was added to surface and patch objects. Additionally, "NormalMode" was renamed to "VertexNormalsMode" and "FaceNormalsMode" was added [1].
In Octave, scripts or functions that try to get or set one of the new properties terminate with an error:
A minimal patch should at least allow to get and set these properties without support for automatic calculation.
I will try and submit a minimal patch when time permits.
[1] http://de.mathworks.com/help/matlab/examples/changing-surface-properties.html#zmw57dd0e5545
|