From: Norbert N. <Nor...@gm...> - 2006-10-09 11:00:20
|
Hi Eric, hi everybody, I just noticed that you, Eric, have committed r2799 to SVN which partly reverts my commit r2790. I understand that, obviously, the changes that I introduced are more controversial than I would have expected, so we should try to reach an agreement. Was your objection generally about my patch or just about some detail? I know that the patch might have broken a bit of compatibility, but since the old logic was inconsistent and hard to understand, I figured that a cleaner and simpler logic would justify a some possible break of compatibility. If you disagree with some change in principle, I would like to know your objection. If there simply was a bug introduced by the change, I appologize and hope I can fix it. In any case, I have to point out that the current state of the SVN code after the partial reversion of my commit is not self-consistent: My commit changed several files besides axes.py - only reverting this one file is bound to cause problems. Greetings, Norbert |
From: Eric F. <ef...@ha...> - 2006-10-09 18:07:29
|
Norbert, Your 2790 patch seems to have completely wiped out the handling of marker colors in the simple case "N.plot(N.random.random(1000),'r.')", as noted by Stefan Van der Walt in a message yesterday to the matplotlib users list. I presume you are monitoring that list, and saw his message and my reply in which I noted that I was reverting the changes to axes.py. I looked at the changes to axes.py, and it appeared that you had simply forgotten the marker color handling; otherwise, the changes appeared to be code rearrangements, not API changes that would interact with other files, so I thought that reverting that file alone would work--that is, restore basic functionality to the svn version. My original thought was to wait and let you fix it, and evidently I should have, but I decided it would be better not to leave svn without the marker color functionality in the interim, and I had no way of knowing whether you would be able to get to it right away. In any case, I made the change, verified that it did restore marker color handling, committed it to svn, and left it at that. Of course I was too hasty, and neglected to look at the patch as a whole; now I see that you were trying to shift the setting of marker colors out of axes.py and into lines.py. I still don't know why Stefan's simple example was broken, and I don't have time now to understand your patch properly, though I wish I did. I certainly don't object to the idea of trying to make the code cleaner and easier to follow. So: please either restore your patch with additional changes to fix the problem Stefan found, or revert all of it cleanly until this can be sorted out. If you think there might be incompatibilities introduced by your patch, then it would be good to know what they are. Sorry to have made the situation more confusing than necessary. Eric Norbert Nemec wrote: > Hi Eric, hi everybody, > > I just noticed that you, Eric, have committed r2799 to SVN which partly > reverts my commit r2790. > > I understand that, obviously, the changes that I introduced are more > controversial than I would have expected, so we should try to reach an > agreement. > > Was your objection generally about my patch or just about some detail? I > know that the patch might have broken a bit of compatibility, but since > the old logic was inconsistent and hard to understand, I figured that a > cleaner and simpler logic would justify a some possible break of > compatibility. > > If you disagree with some change in principle, I would like to know your > objection. If there simply was a bug introduced by the change, I > appologize and hope I can fix it. > > In any case, I have to point out that the current state of the SVN code > after the partial reversion of my commit is not self-consistent: My > commit changed several files besides axes.py - only reverting this one > file is bound to cause problems. > > Greetings, > Norbert |
From: John H. <jdh...@ac...> - 2006-10-10 03:36:51
|
>>>>> "Eric" == Eric Firing <ef...@ha...> writes: Eric> So: please either restore your patch with additional changes Eric> to fix the problem Stefan found, or revert all of it cleanly Eric> until this can be sorted out. Norbert -- can you describe briefly the inconsistencies in the format arg color handling you were trying to address in your patch? I see this -- http://sourceforge.net/tracker/index.php?func=detail&aid=1504387&group_id=80706&atid=560722 -- but not much detail about the problems the changes fix. Also, for all developers, when making API changes * post here with a brief descriptions of changes and rationale * make a note in API_CHANGES * run backend_driver.py. It is a good habit to not only run the script to make sure no exceptions are raised, but also to view the output PNG etc files in your favorite image viewer to make sure the output is correct Thanks! JDH |
From: Norbert N. <Nor...@gm...> - 2006-10-10 07:17:58
|
OK, I found the problem and committed a temporary fix. The real problem, however is rooted a bit deeper. First an explanation of the intended change: It used to be that marker colors were partly automatic, but not completely. I.e. plot(x,y,'-or') would set both, line color and marker color to red. However plot(x,y,color='r') would set only the line color and leave marker color to the default. My change was to introduce a special value 'auto' for markeredgecolor and markerfacecolor. This special value would cause the marker color to always follow the line color. (Which is, in 99% of the cases, what you want.) Most of the special logic in axes.py could therefore go away. mfc and mec would simply be left at 'auto' unless explicitely assigned another color. The handling of the special value would then happen in lines.py at time of plotting. (including the effect that for filled markers, the edge would default to black) The problem in r2790: I changed the default value in matplotlibrc to 'auto' and everything worked fine for me. I forgot that, of course, anybody updating from an older version, would still have the values 'blue' and 'black' in their matplotlibrc, which would not be overridden by the '.r' option that Stefan used. The (temporary) solution in r2800: I deactivated the rcfile-configurability of markeredgecolor and markerfacecolor. Assuming that hardly anybody would want to change the 'auto' behavior in their rcfile, this should be a good solution until we have solved the core problem. The core problem: The matplotlibrc file distributed with matplotlib contains all the default values in non-commented lines. This file is usually copied to the home-directory of any user, making it impossible to simply change any default value in later versions. It is not possible to find out which values in the users matplotlibrc were set on purpose and which were just left untouched from the distributed file. The better solution: place '#' at the beginning of every line in matplotlibrc.template (except for 'backend' and 'numerix' which carry important information) Any user who explicitely wants to change a value, can simply uncomment the line and set a value. Otherwise, the default value from matplotlib/__init__.py will remain active, even if changed in an update. Of course, this would only make sense, if users were informed and encouraged to replace their personal matplotlibrc The ultimate solution: The file matplotlib.template should probably be dropped completely and be auto-created from the information in matplotlib/__init__.py - this would remove quite some redundancy and potential for inconsistencies. Comments on this? Greetings, Norbert John Hunter wrote: >>>>>> "Eric" == Eric Firing <ef...@ha...> writes: >>>>>> > > Eric> So: please either restore your patch with additional changes > Eric> to fix the problem Stefan found, or revert all of it cleanly > Eric> until this can be sorted out. > > Norbert -- can you describe briefly the inconsistencies in the format > arg color handling you were trying to address in your patch? I see > this -- > http://sourceforge.net/tracker/index.php?func=detail&aid=1504387&group_id=80706&atid=560722 > -- but not much detail about the problems the changes fix. > > Also, for all developers, when making API changes > > * post here with a brief descriptions of changes and rationale > > * make a note in API_CHANGES > > * run backend_driver.py. It is a good habit to not only run the > script to make sure no exceptions are raised, but also to view > the output PNG etc files in your favorite image viewer to make > sure the output is correct > > > Thanks! > JDH > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share your > opinions on IT & business topics through brief surveys -- and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Matplotlib-devel mailing list > Mat...@li... > https://lists.sourceforge.net/lists/listinfo/matplotlib-devel > > > |
From: Norbert N. <Nor...@gm...> - 2006-10-10 05:28:34
|
OK, I understand. I will try sort out the problem today or otherwise take back the whole patch. I had not seen the message in matplotlib-users because of the backlog of 400 messages that have kept piling up... Don't worry about additional confusion. Quickly reverting the patch was the best thing to do for the moment. Greetings, Norbert Eric Firing wrote: > Norbert, > > Your 2790 patch seems to have completely wiped out the handling of > marker colors in the simple case "N.plot(N.random.random(1000),'r.')", > as noted by Stefan Van der Walt in a message yesterday to the matplotlib > users list. I presume you are monitoring that list, and saw his message > and my reply in which I noted that I was reverting the changes to > axes.py. I looked at the changes to axes.py, and it appeared that you > had simply forgotten the marker color handling; otherwise, the changes > appeared to be code rearrangements, not API changes that would interact > with other files, so I thought that reverting that file alone would > work--that is, restore basic functionality to the svn version. My > original thought was to wait and let you fix it, and evidently I should > have, but I decided it would be better not to leave svn without the > marker color functionality in the interim, and I had no way of knowing > whether you would be able to get to it right away. In any case, I made > the change, verified that it did restore marker color handling, > committed it to svn, and left it at that. > > Of course I was too hasty, and neglected to look at the patch as a > whole; now I see that you were trying to shift the setting of marker > colors out of axes.py and into lines.py. I still don't know why > Stefan's simple example was broken, and I don't have time now to > understand your patch properly, though I wish I did. I certainly don't > object to the idea of trying to make the code cleaner and easier to follow. > > So: please either restore your patch with additional changes to fix the > problem Stefan found, or revert all of it cleanly until this can be > sorted out. > > If you think there might be incompatibilities introduced by your patch, > then it would be good to know what they are. > > Sorry to have made the situation more confusing than necessary. > > Eric > > > > > Norbert Nemec wrote: > >> Hi Eric, hi everybody, >> >> I just noticed that you, Eric, have committed r2799 to SVN which partly >> reverts my commit r2790. >> >> I understand that, obviously, the changes that I introduced are more >> controversial than I would have expected, so we should try to reach an >> agreement. >> >> Was your objection generally about my patch or just about some detail? I >> know that the patch might have broken a bit of compatibility, but since >> the old logic was inconsistent and hard to understand, I figured that a >> cleaner and simpler logic would justify a some possible break of >> compatibility. >> >> If you disagree with some change in principle, I would like to know your >> objection. If there simply was a bug introduced by the change, I >> appologize and hope I can fix it. >> >> In any case, I have to point out that the current state of the SVN code >> after the partial reversion of my commit is not self-consistent: My >> commit changed several files besides axes.py - only reverting this one >> file is bound to cause problems. >> >> Greetings, >> Norbert >> > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share your > opinions on IT & business topics through brief surveys -- and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Matplotlib-devel mailing list > Mat...@li... > https://lists.sourceforge.net/lists/listinfo/matplotlib-devel > > > |
From: Eric F. <ef...@ha...> - 2006-10-10 18:11:46
|
Norbert, I am splitting this thread because I think different issues are involved in different parts. > >> In looking at your original patch, I also wondered what is the reason >> for supporting 3 different ways of specifying "_draw_nothing"? (I had >> not previously noticed that there was any such thing at all, and I guess >> I don't understand what it is for.) >> class Line2D(Artist): >> _lineStyles = { >> '-' : '_draw_solid', >> '--' : '_draw_dashed', >> '-.' : '_draw_dash_dot', >> ':' : '_draw_dotted', >> 'steps': '_draw_steps', >> 'None' : '_draw_nothing', >> ' ' : '_draw_nothing', >> '' : '_draw_nothing', >> } >> > The idea was to be able to say something like > plot(x,y,line='',marker='o',color=(.2,.5,.8)) > which seemed a bit more intuitive to me than line='None' when I created > the patch. By now, I wonder about it myself... It raises larger API questions. From the standpoint of user-level code readability, the present array of marker and line identifiers (inherited from Matlab) is not good. For example, why should '-' mean a solid line, but '_' means a horizontal line marker? What is the difference between '^' and '2', and how on earth is anyone reading the code supposed to know what '2' means? The only justification I can see for the 1- and 2-character codes is their convenience in interactive use for things like "plot(x,y,'g.'". I would not want this to go away, but for non-interactive coding I think longer names, typically corresponding to the name of the function that generates the marker (in the case of markers) should be available and used. Further, it may be that only the set of unique descriptive names should be available at the level of lines.py, and the shorthand versions should be relegated to support code for the plot command, which is at a higher level. Going even further, maybe the short format strings should be limited to a subset of available lines and markers. Is it really important to let people write "plot(x,y,'g2')" instead of "plot(x,y, color='g', marker='tri_up')"? (I still don't know what is the difference between "tri_up" and "triangle_up".) This needs either some broader discussion, or a command decision by John along the lines of "Forget it; it's OK the way it is, or it is too late to make any change in this part of the API." Eric |
From: John H. <jdh...@ac...> - 2006-10-10 18:19:13
|
>>>>> "Eric" == Eric Firing <ef...@ha...> writes: Eric> marker='tri_up')"? (I still don't know what is the Eric> difference between "tri_up" and "triangle_up".) tri_up is a tripod up, vs a triangle up. JDH |
From: John H. <jdh...@ac...> - 2006-10-10 18:34:37
|
>>>>> "Eric" == Eric Firing <ef...@ha...> writes: Eric> It raises larger API questions. From the standpoint of Eric> user-level code readability, the present array of marker and Eric> line identifiers (inherited from Matlab) is not good. For Eric> example, why should '-' mean a solid line, but '_' means a Eric> horizontal line marker? What is the difference between '^' Eric> and '2', and how on earth is anyone reading the code Eric> supposed to know what '2' means? The only justification I Eric> can see for the 1- and 2-character codes is their Eric> convenience in interactive use for things like Eric> "plot(x,y,'g.'". I would not want this to go away, but for Eric> non-interactive coding I think longer names, typically Eric> corresponding to the name of the function that generates the I think for interactive use it is important to keep these short characters though you are right, for some of the more arcane ones like 1,2,3,4 it makes no sense since there is no mnemonic. But I see no harm in keeping them for interactive users Eric> This needs either some broader discussion, or a command Eric> decision by John along the lines of "Forget it; it's OK the Eric> way it is, or it is too late to make any change in this part Eric> of the API." As you suggest, what we need to provide is an easy way for people to use long readable names. The easiest way is in set_marker and set_linestyle to do something like (untested sketch) def set_linestyle(self, linestyle): funcname = '_draw_%s'%linestyle func = getattr(self, funcname, None) if callable(func): self._lineFunc = func and ditto for set_marker while still supporting the old mnemonics. We can then advertise any of names available as values in the Line2D._lineStyles and Line2D._markers dictionaries as legitimate values for the linestyle and marker properties. Extra credit for auto-updating the docstring by parsing the dictionary values. In [1]: from matplotlib.lines import Line2D In [2]: print [name[6:] for name in Line2D._markers.values() if name.startswith('_draw_')] ['tickleft', 'tickright', 'tickup', 'tickdown', 'nothing', 'nothing', 'plus', 'pixel', 'point', 'tri_down', 'tri_left', 'tri_up', 'tri_right', 'triangle_left', 'triangle_right', 'nothing', 'diamond', 'hexagon2', 'hline', 'triangle_up', 'thin_diamond', 'hexagon1', 'circle', 'pentagon', 'square', 'triangle_down', 'x', 'vline'] JDH |
From: Eric F. <ef...@ha...> - 2006-10-10 18:31:44
|
Norbert, >>> The core problem: The matplotlibrc file distributed with matplotlib >>> contains all the default values in non-commented lines. This file is >>> usually copied to the home-directory of any user, making it impossible >>> to simply change any default value in later versions. It is not possible >>> to find out which values in the users matplotlibrc were set on purpose >>> and which were just left untouched from the distributed file. >>> >>> The better solution: place '#' at the beginning of every line in >>> matplotlibrc.template (except for 'backend' and 'numerix' which carry >>> important information) Any user who explicitely wants to change a value, >>> can simply uncomment the line and set a value. Otherwise, the default >>> value from matplotlib/__init__.py will remain active, even if changed in >>> an update. Of course, this would only make sense, if users were informed >>> and encouraged to replace their personal matplotlibrc >>> >> This seems like a good idea, and one that is consistent with the way >> many other configurable systems are often handled. I think that >> regardless of what else is done, this would reduce pain during updates; >> it would also make it easier for the user to see what changes to the >> defaults he/she has made. >> > So, should we simply do that? The only problem that I see is, that > matplotlibrc.template will probably soon be out of sync with > defaultParams in __init__.py, once there is no necessity for developers > to update it. > >>> The ultimate solution: The file matplotlib.template should probably be >>> dropped completely and be auto-created from the information in >>> matplotlib/__init__.py - this would remove quite some redundancy and >>> potential for inconsistencies. >>> >> Reducing redundancy is appealing, but I don't know if it would be worth >> the effort of implementing your auto-generation idea--which might add >> clutter and complexity to __init__.py. >> > Not necessarily. I was thinking of moving defaultParams out of > __init__.py to a separate file, which can be imported by setup.py to > write matplotlibrc. This would even reduce the complexity of > __init__.py. However, it will need some cleanup first to reduce > dependencies. I've started on that, but it will take some more time. > > In any case, this auto-generation would solve the problem of a > increasingly outdated matplotlibrc.template. Before going too far with all this, it would be good to hear from John and others. It all sounds good to me, but others might have objections or good alternatives that should be considered. To summarize, your two proposals are: 1) generate matplotlibrc with almost everything commented out by default 2) eliminate matplotlibrc.template by having setup.py autogenerate matplotlibrc based on the rcParams-related code that is presently in __init__.py. Eric |
From: Norbert N. <Nor...@gm...> - 2006-10-11 15:17:09
|
Eric Firing wrote: > Norbert, > >>> >>> >>>> The problem in r2790: I changed the default value in matplotlibrc to >>>> 'auto' and everything worked fine for me. I forgot that, of course, >>>> anybody updating from an older version, would still have the values >>>> 'blue' and 'black' in their matplotlibrc, which would not be overridden >>>> by the '.r' option that Stefan used. >>>> >>>> >>> This is not the first time matplotlibrc has bitten us, and it won't be >>> the last... >>> >>> But *shouldn't* '.r' override a setting in matplotlibrc, regardless of >>> what that setting is? I think it should have set the mfc, or preferably >>> both the mfc and the mec. >>> >>> >> OK, that would be an alternative solution: set both mfc and mec to >> 'auto', whenever the color is specified using a format string. However, >> this would mean that the rcfile options markeredgecolor and >> markerfacecolor are often ignored, even if they were set on purpose. If >> that is the case, one could just as well deactivate them completely and >> prevent some confusion. >> >> > > Either I am not understanding you correctly, or we have fundamentally > different views of the role of matplotlibrc values. The way I see it, > function args and kwargs *always* override matplotlibrc values, which in > turn *always* override built-in defaults. So in the example above, if > the user writes "plot(x,y,'r.')", red dots should be plotted no matter > what is in matplotlibrc. It should not depend on whether something is > set to 'auto'. > I fully agree with you. Guess, the simple solution to the dilemma is to drop the idea of rcfile-configurability of markerfacecolor and markeredgecolor. That way, markers would always have the same color as the line, unless explicitely set differently by the kwargs markeredgecolor/markerfacecolor. This is, what is wanted in 99% of the cases and for every other case, a rcfile-option will not help anyway. Greetings, Norbert |
From: Eric F. <ef...@ha...> - 2006-10-11 17:58:02
|
Norbert, >>Either I am not understanding you correctly, or we have fundamentally >>different views of the role of matplotlibrc values. The way I see it, >>function args and kwargs *always* override matplotlibrc values, which in >>turn *always* override built-in defaults. So in the example above, if >>the user writes "plot(x,y,'r.')", red dots should be plotted no matter >>what is in matplotlibrc. It should not depend on whether something is >>set to 'auto'. >> > > > I fully agree with you. > > Guess, the simple solution to the dilemma is to drop the idea of > rcfile-configurability of markerfacecolor and markeredgecolor. That way, > markers would always have the same color as the line, unless explicitely > set differently by the kwargs markeredgecolor/markerfacecolor. > > This is, what is wanted in 99% of the cases and for every other case, a > rcfile-option will not help anyway. This sounds ideal to me--it makes everything simpler, both in the code and in explaining what the behavior is. Thanks! Eric |
From: Norbert N. <Nor...@gm...> - 2006-10-11 19:55:55
|
John Hunter wrote: >>>>>> "Eric" == Eric Firing <ef...@ha...> writes: >>>>>> > > >> This is, what is wanted in 99% of the cases and for every other > >> case, a rcfile-option will not help anyway. > > Eric> This sounds ideal to me--it makes everything simpler, both > Eric> in the code and in explaining what the behavior is. > > Hmm.... I can imagine that there are those who want the default > markeredgecolor to be the same color as the facecolor, and those who > want the default edgecolor to be black regardless of facecolor. I am > a bit hesitant to pull this functionality, though I agree that simpler > is better. > This functionality was never there, so nobody can miss it. Before my changes, the options in matplotlibrc only allowed to specify fixed colors for mfc and mec. This is now not possible any more, but can easily be done via kwargs. Automatic coloring was just as inflexible as it is now but less consistent. I thought about this kind of configurability, but any clean solution that I could find, would have become awfully complex. |
From: Eric F. <ef...@ha...> - 2006-10-10 08:11:51
|
Norbert Nemec wrote: > OK, I found the problem and committed a temporary fix. The real problem, > however is rooted a bit deeper. > > First an explanation of the intended change: > > It used to be that marker colors were partly automatic, but not > completely. I.e. > > plot(x,y,'-or') > > would set both, line color and marker color to red. However > > plot(x,y,color='r') > > would set only the line color and leave marker color to the default. > My change was to introduce a special value 'auto' for markeredgecolor and > markerfacecolor. This special value would cause the marker color to > always follow > the line color. (Which is, in 99% of the cases, what you want.) Most of > the special logic in axes.py could therefore go away. mfc and mec would > simply be left at 'auto' unless explicitely assigned another color. The > handling of the special value would then happen in lines.py at time of > plotting. (including the effect that for filled markers, the edge would > default to black) > In your explanation above, it is not clear what happens in each of the 4 cases: mec auto or non-auto, and mfc auto or non-auto. In looking at your original patch, I also wondered what is the reason for supporting 3 different ways of specifying "_draw_nothing"? (I had not previously noticed that there was any such thing at all, and I guess I don't understand what it is for.) class Line2D(Artist): _lineStyles = { '-' : '_draw_solid', '--' : '_draw_dashed', '-.' : '_draw_dash_dot', ':' : '_draw_dotted', 'steps': '_draw_steps', 'None' : '_draw_nothing', ' ' : '_draw_nothing', '' : '_draw_nothing', } I was also a little uncomfortable with pushing some of the color decision logic all the way down into the draw method, together with a default value, although maybe there is no better way to get the desired behavior: if self._marker is not None: gc = renderer.new_gc() if (is_string_like(self._markeredgecolor) and self._markeredgecolor == 'auto'): if self._marker in self.filled_markers: gc.set_foreground('k') else: gc.set_foreground(self._color) else: gc.set_foreground(self._markeredgecolor) > The problem in r2790: I changed the default value in matplotlibrc to > 'auto' and everything worked fine for me. I forgot that, of course, > anybody updating from an older version, would still have the values > 'blue' and 'black' in their matplotlibrc, which would not be overridden > by the '.r' option that Stefan used. This is not the first time matplotlibrc has bitten us, and it won't be the last... But *shouldn't* '.r' override a setting in matplotlibrc, regardless of what that setting is? I think it should have set the mfc, or preferably both the mfc and the mec. > > The (temporary) solution in r2800: I deactivated the > rcfile-configurability of markeredgecolor and markerfacecolor. Assuming > that hardly anybody would want to change the 'auto' behavior in their > rcfile, this should be a good solution until we have solved the core > problem. > > The core problem: The matplotlibrc file distributed with matplotlib > contains all the default values in non-commented lines. This file is > usually copied to the home-directory of any user, making it impossible > to simply change any default value in later versions. It is not possible > to find out which values in the users matplotlibrc were set on purpose > and which were just left untouched from the distributed file. > > The better solution: place '#' at the beginning of every line in > matplotlibrc.template (except for 'backend' and 'numerix' which carry > important information) Any user who explicitely wants to change a value, > can simply uncomment the line and set a value. Otherwise, the default > value from matplotlib/__init__.py will remain active, even if changed in > an update. Of course, this would only make sense, if users were informed > and encouraged to replace their personal matplotlibrc This seems like a good idea, and one that is consistent with the way many other configurable systems are often handled. I think that regardless of what else is done, this would reduce pain during updates; it would also make it easier for the user to see what changes to the defaults he/she has made. > > The ultimate solution: The file matplotlib.template should probably be > dropped completely and be auto-created from the information in > matplotlib/__init__.py - this would remove quite some redundancy and > potential for inconsistencies. Reducing redundancy is appealing, but I don't know if it would be worth the effort of implementing your auto-generation idea--which might add clutter and complexity to __init__.py. Eric |
From: Norbert N. <Nor...@gm...> - 2006-10-10 11:56:35
|
Eric Firing wrote: > Norbert Nemec wrote: > >> OK, I found the problem and committed a temporary fix. The real problem, >> however is rooted a bit deeper. >> >> First an explanation of the intended change: >> >> It used to be that marker colors were partly automatic, but not >> completely. I.e. >> >> plot(x,y,'-or') >> >> would set both, line color and marker color to red. However >> >> plot(x,y,color='r') >> >> would set only the line color and leave marker color to the default. >> My change was to introduce a special value 'auto' for markeredgecolor and >> markerfacecolor. This special value would cause the marker color to >> always follow >> the line color. (Which is, in 99% of the cases, what you want.) Most of >> the special logic in axes.py could therefore go away. mfc and mec would >> simply be left at 'auto' unless explicitely assigned another color. The >> handling of the special value would then happen in lines.py at time of >> plotting. (including the effect that for filled markers, the edge would >> default to black) >> >> > > In your explanation above, it is not clear what happens in each of the 4 > cases: mec auto or non-auto, and mfc auto or non-auto. > mec and mfc are handled independently. markeredges are drawn using: mec, if mec!='auto' color, if mec=='auto' and marker not in filled_markers black, if mec=='auto' and marker in filled_marker markerfaces are drawn using: mfc, if mfc!='auto' color, if mfc=='auto' Since non-filled markers are considered 'edges' without filling, this logic is necessary to get the correct behavior. > In looking at your original patch, I also wondered what is the reason > for supporting 3 different ways of specifying "_draw_nothing"? (I had > not previously noticed that there was any such thing at all, and I guess > I don't understand what it is for.) > class Line2D(Artist): > _lineStyles = { > '-' : '_draw_solid', > '--' : '_draw_dashed', > '-.' : '_draw_dash_dot', > ':' : '_draw_dotted', > 'steps': '_draw_steps', > 'None' : '_draw_nothing', > ' ' : '_draw_nothing', > '' : '_draw_nothing', > } > The idea was to be able to say something like plot(x,y,line='',marker='o',color=(.2,.5,.8)) which seemed a bit more intuitive to me than line='None' when I created the patch. By now, I wonder about it myself... If there is an objection against this detail, I can revert it. I probably should have split the patch in parts in the first place. Unfortunately, the different parts had become more interdependent than I would have liked. > I was also a little uncomfortable with pushing some of the color > decision logic all the way down into the draw method, together with a > default value, although maybe there is no better way to get the desired > behavior: > > if self._marker is not None: > gc = renderer.new_gc() > if (is_string_like(self._markeredgecolor) and > self._markeredgecolor == 'auto'): > if self._marker in self.filled_markers: > gc.set_foreground('k') > else: > gc.set_foreground(self._color) > else: > gc.set_foreground(self._markeredgecolor) > This was the cleanest solution. If the decision is made earlier, one always has to store not only the value of mec, but also, whether it was set explicitely or automatically. Otherwise, the marker color is not updated when set_color is called on an existing graph. (Which was the problem that started my whole effort) > > > >> The problem in r2790: I changed the default value in matplotlibrc to >> 'auto' and everything worked fine for me. I forgot that, of course, >> anybody updating from an older version, would still have the values >> 'blue' and 'black' in their matplotlibrc, which would not be overridden >> by the '.r' option that Stefan used. >> > > This is not the first time matplotlibrc has bitten us, and it won't be > the last... > > But *shouldn't* '.r' override a setting in matplotlibrc, regardless of > what that setting is? I think it should have set the mfc, or preferably > both the mfc and the mec. > OK, that would be an alternative solution: set both mfc and mec to 'auto', whenever the color is specified using a format string. However, this would mean that the rcfile options markeredgecolor and markerfacecolor are often ignored, even if they were set on purpose. If that is the case, one could just as well deactivate them completely and prevent some confusion. >> The (temporary) solution in r2800: I deactivated the >> rcfile-configurability of markeredgecolor and markerfacecolor. Assuming >> that hardly anybody would want to change the 'auto' behavior in their >> rcfile, this should be a good solution until we have solved the core >> problem. >> >> The core problem: The matplotlibrc file distributed with matplotlib >> contains all the default values in non-commented lines. This file is >> usually copied to the home-directory of any user, making it impossible >> to simply change any default value in later versions. It is not possible >> to find out which values in the users matplotlibrc were set on purpose >> and which were just left untouched from the distributed file. >> >> The better solution: place '#' at the beginning of every line in >> matplotlibrc.template (except for 'backend' and 'numerix' which carry >> important information) Any user who explicitely wants to change a value, >> can simply uncomment the line and set a value. Otherwise, the default >> value from matplotlib/__init__.py will remain active, even if changed in >> an update. Of course, this would only make sense, if users were informed >> and encouraged to replace their personal matplotlibrc >> > > This seems like a good idea, and one that is consistent with the way > many other configurable systems are often handled. I think that > regardless of what else is done, this would reduce pain during updates; > it would also make it easier for the user to see what changes to the > defaults he/she has made. > So, should we simply do that? The only problem that I see is, that matplotlibrc.template will probably soon be out of sync with defaultParams in __init__.py, once there is no necessity for developers to update it. >> The ultimate solution: The file matplotlib.template should probably be >> dropped completely and be auto-created from the information in >> matplotlib/__init__.py - this would remove quite some redundancy and >> potential for inconsistencies. >> > > Reducing redundancy is appealing, but I don't know if it would be worth > the effort of implementing your auto-generation idea--which might add > clutter and complexity to __init__.py. > Not necessarily. I was thinking of moving defaultParams out of __init__.py to a separate file, which can be imported by setup.py to write matplotlibrc. This would even reduce the complexity of __init__.py. However, it will need some cleanup first to reduce dependencies. I've started on that, but it will take some more time. In any case, this auto-generation would solve the problem of a increasingly outdated matplotlibrc.template. |
From: Eric F. <ef...@ha...> - 2006-10-10 18:16:33
|
Norbert, >> >>> The problem in r2790: I changed the default value in matplotlibrc to >>> 'auto' and everything worked fine for me. I forgot that, of course, >>> anybody updating from an older version, would still have the values >>> 'blue' and 'black' in their matplotlibrc, which would not be overridden >>> by the '.r' option that Stefan used. >>> >> This is not the first time matplotlibrc has bitten us, and it won't be >> the last... >> >> But *shouldn't* '.r' override a setting in matplotlibrc, regardless of >> what that setting is? I think it should have set the mfc, or preferably >> both the mfc and the mec. >> > OK, that would be an alternative solution: set both mfc and mec to > 'auto', whenever the color is specified using a format string. However, > this would mean that the rcfile options markeredgecolor and > markerfacecolor are often ignored, even if they were set on purpose. If > that is the case, one could just as well deactivate them completely and > prevent some confusion. > Either I am not understanding you correctly, or we have fundamentally different views of the role of matplotlibrc values. The way I see it, function args and kwargs *always* override matplotlibrc values, which in turn *always* override built-in defaults. So in the example above, if the user writes "plot(x,y,'r.')", red dots should be plotted no matter what is in matplotlibrc. It should not depend on whether something is set to 'auto'. Eric |
From: John H. <jdh...@ac...> - 2006-10-10 18:41:18
|
>>>>> "Eric" == Eric Firing <ef...@ha...> writes: Eric> 1) generate matplotlibrc with almost everything commented Eric> out by default +2 Hopefully, this will address the problem of all the deprecated rc warnings people are getting, which is confusing to new users. Eric> 2) eliminate matplotlibrc.template by having setup.py Eric> autogenerate matplotlibrc based on the rcParams-related code Eric> that is presently in __init__.py. working code tested across platforms and python versions settles this. It is mostly working now, but Norbert brings up a good point that if we now go to mostly empty rc files, it will become increasingly unlikely that the template gets out of whack. If this is important to someone and they can come up with a good implementation, I don't have a problem with it. JDH |
From: Norbert N. <Nor...@gm...> - 2006-10-11 16:01:29
|
John Hunter wrote: >>>>>> "Eric" == Eric Firing <ef...@ha...> writes: >>>>>> > > Eric> 1) generate matplotlibrc with almost everything commented > Eric> out by default > > +2 > > Hopefully, this will address the problem of all the deprecated rc > warnings people are getting, which is confusing to new users. > As you saw, I boldly implemented that change already. > Eric> 2) eliminate matplotlibrc.template by having setup.py > Eric> autogenerate matplotlibrc based on the rcParams-related code > Eric> that is presently in __init__.py. > > working code tested across platforms and python versions settles this. > It is mostly working now, but Norbert brings up a good point that if > we now go to mostly empty rc files, it will become increasingly > unlikely that the template gets out of whack. If this is important to > someone and they can come up with a good implementation, I don't have > a problem with it. > I already started working on this. Don't know when I find the time to finish, but I will post patches for testing as soon as I have something clean enough. Greetings, Norbert |
From: John H. <jdh...@ac...> - 2006-10-11 18:04:18
|
>>>>> "Eric" == Eric Firing <ef...@ha...> writes: >> This is, what is wanted in 99% of the cases and for every other >> case, a rcfile-option will not help anyway. Eric> This sounds ideal to me--it makes everything simpler, both Eric> in the code and in explaining what the behavior is. Hmm.... I can imagine that there are those who want the default markeredgecolor to be the same color as the facecolor, and those who want the default edgecolor to be black regardless of facecolor. I am a bit hesitant to pull this functionality, though I agree that simpler is better. JDH |
From: Eric F. <ef...@ha...> - 2006-10-11 18:31:35
|
John Hunter wrote: >>>>>>"Eric" == Eric Firing <ef...@ha...> writes: > > > >> This is, what is wanted in 99% of the cases and for every other > >> case, a rcfile-option will not help anyway. > > Eric> This sounds ideal to me--it makes everything simpler, both > Eric> in the code and in explaining what the behavior is. > > Hmm.... I can imagine that there are those who want the default > markeredgecolor to be the same color as the facecolor, and those who > want the default edgecolor to be black regardless of facecolor. I am > a bit hesitant to pull this functionality, though I agree that simpler > is better. How about replacing the markeredgecolor and markerfacecolor rc options (but not the kwargs) with something like this: markeredgedefault = 'face' | colorspec If something like this is chosen, I think it should apply only to filled markers. Here is a variation on the theme: markeredgedefault = colorspec where colorspec can include 'None' and means "don't draw it". I think that what we actually want for filled markers with the edge color matching the face is not to set the edgecolor to the facecolor, but to not draw the edge at all; this will render better and be more efficient at all levels. (I suspect the 'None' colorspec should be uniformly supported all the way from the high level down to the backends. That would eliminate high-level checking for it as a special case.) As part of this, I think we should be thinking of the "marker color" as the face color for filled markers and as the line color for non-filled markers; for filled markers, the edge is better thought of as the "outline", which is missing for line markers. The clearest point in all of this seems to be that trying to have a 1:1 relation between kwargs and rc params is inconsistent with achieving nice default behavior in this case. Sorry this is a bit of a ramble but I am short on time right now. Norbert, can we take a few days if necessary to think this through carefully and make sure John and others are comfortable with the whole plan before going any further? Eric |
From: John H. <jdh...@ac...> - 2006-10-11 20:07:04
|
>>>>> "Norbert" == Norbert Nemec <Nor...@gm...> writes: Norbert> This functionality was never there, so nobody can miss Norbert> it. Before my changes, the options in matplotlibrc only Norbert> allowed to specify fixed colors for mfc and mec. This is Norbert> now not possible any more, but can easily be done via Norbert> kwargs. Automatic coloring was just as inflexible as it Norbert> is now but less consistent. Yeah, I mispoke a bit. What I meant is that I prefer black edges, and I expect plot(rand(10), 'go') to have a green face and black edges. There is no way in the new infrastructure for this to happen by default as far as I can see, but I can pass mec if I want. I can live with it, but I may not be the only one, so be prepared for griping. Or we can consider something like Eric proposed where mec can either follow mfc or be set to a fixed color, or something along those lines. JDH |
From: John H. <jdh...@ac...> - 2006-10-11 20:15:47
|
On another note, I get messages like mpl/examples> python simple_plot.py -dAgg Bad key "lines.markeredgecolor" on line 48 in /home/jdhunter/.matplotlib/matplotlibrc. You probably need to get an updated matplotlibrc file from http://matplotlib.sf.net/matplotlibrc or from the matplotlib source distribution Bad key "lines.markerfacecolor" on line 47 in /home/jdhunter/.matplotlib/matplotlibrc. You probably need to get an updated matplotlibrc file from http://matplotlib.sf.net/matplotlibrc or from the matplotlib source distribution since I did yet not update to the new rc which is to be expected. I know from experience that neophyte users are confused by this. Many mpl users don't even know that an rc file exists, what it is for, and how to find it. While you are mucking around in rc and __init__.py, you might consider a more helpful deprecation message scheme, with part of the message boilerplate and part of the message specific to the bad rc key. Eg, if a user has lines.markerfacecolor in his rc: The setting "lines.markerfacecolor" in your parameter file /home/jdhunter/.matplotlib/matplotlibrc is deprecated in this version of matplotlib. The configuration for marker facecolors was your was recently changed. blah blah blah explain the new interface and behavior. You may want to replace /home/jdhunter/.matplotlib/matplotlibrc with the latest file http://matplotlib.sf.net/matplotlibrc . Something to think about. JDH |
From: Norbert N. <Nor...@gm...> - 2006-10-12 05:00:47
|
John Hunter wrote: >>>>>> "Norbert" == Norbert Nemec <Nor...@gm...> writes: >>>>>> > Norbert> This functionality was never there, so nobody can miss > Norbert> it. Before my changes, the options in matplotlibrc only > Norbert> allowed to specify fixed colors for mfc and mec. This is > Norbert> now not possible any more, but can easily be done via > Norbert> kwargs. Automatic coloring was just as inflexible as it > Norbert> is now but less consistent. > > Yeah, I mispoke a bit. What I meant is that I prefer black edges, and > I expect > > plot(rand(10), 'go') > > to have a green face and black edges. There is no way in the new > infrastructure for this to happen by default as far as I can see, but > I can pass mec if I want. > Actually, this is the new default behavior: for filled markers, the mfc is set to the line color and the mec is set to black. For non-filled markers, mec is set to the line color and mfc is not used at all. What is impossible to set by default are alternative settings like * mec and mfc to line color * mec to line color and mfc to white > I can live with it, but I may not be the only one, so be prepared for > griping. Or we can consider something like Eric proposed where mec > can either follow mfc or be set to a fixed color, or something along > those lines. > > JDH > > ------------------------------------------------------------------------- > Using Tomcat but need to do more? Need to support web services, security? > Get stuff done quickly with pre-integrated technology to make your job easier > Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 > _______________________________________________ > Matplotlib-devel mailing list > Mat...@li... > https://lists.sourceforge.net/lists/listinfo/matplotlib-devel > > > |