Re: [matplotlib-devel] strategy for 1.2.x, master, PEP8 changes
> In the meantime, PEP8 PRs can be completed on master, after which feature enhancements can proceed on master. I think it would be helpful to hold fire on the PEP8 changes until we have our rc3 and have merged it back onto master for (hopefully) the last time. That way, we wont have to deal with the conflicts that are inevitable when merging v1.2.x back to master. > Any merge conflicts with the PEP8 work will be noted by git and we can address them as they come. That is another approach which will work, assuming the master is fairly up-to-date with v1.2.x now. > Any strong objections to this? Nope. Sounds like a good plan. > In short, provided we can agree a future matplotlib version schedule, I agree with Eric. Mike, Ben, Eric & others, there is a new thread relating to scheduling of releases, it is my opinion that it would be really beneficial to have a planned release date for v1.3 (and maybe even v1.4/v2.0) that we all agree on. Phil On 15 October 2012 17:59, Michael Droettboom wrote: > On 10/15/2012 12:52 PM, Eric Firing wrote: > > On 2012/10/15 5:50 AM, Michael Droettboom wrote: > >> Sorry to be jumping in on this late -- I didn't have a chance to catch > >> up with this over the weekend. > >> > >> I think I generally side with Eric here -- the rc candidate cycle is > >> intended to be quite conservative. Nelle's pull requests are very > >> welcome improvements, but they are also quite large and I am concerned > >> about breakage slipping through the cracks. To the extent that Nelle is > >> finding undefined variable bugs etc. with her tool, I think we should > >> probably try and fix those -- I know we've been doing that already and > >> that's great. > >> > >> I think we should take the 1.2.x milestone off of all of the PEP8 > >> changes and keep all of them on master going forward. Yes, the merging > >> may be difficult while we are still in maintaining 1.2.x, but I think > >> that's trivial compared to all of the additional testing and push back > >> of the 1.2.0 release that this is currently causing. > >> > >> As for backing out things that were already cherry-picked -- that's a > >> tough call. I don't want to exacerbate the situation by causing further > >> risk. Maybe we just back out everything since the rc2? > > It looks like that would itself cause a huge amount of additional churn, > > and more risk than leaving things as they are. At this point I suggest > > leaving everything in that is presently in, try to get in the last few > > bug fixes and tweaks, tag an rc3, and then target a release date, > > somewhere in in the 2-4 weeks hence range. In the meantime, PEP8 PRs > > can be completed on master, after which feature enhancements can proceed > > on master. > > Yeah -- having just looked back at all of the cherry-picks at issue, > that's where I've come down as well. Let's leave them in, but not put > any further PEP8-only fixes on 1.2.x. I'll remove the 1.2.x issue > labels on the few PRs in progress. > > I also think it's not the end of the world if additional feature > enhancements go on in master in the meantime. Any merge conflicts with > the PEP8 work will be noted by git and we can address them as they come. > > Any strong objections to this? > > Mike > > > > > Eric > > > >> Mike > >> > >> On 10/15/2012 12:10 AM, Eric Firing wrote: > >>> On 2012/10/14 12:44 PM, Damon McDougall wrote: > On Sun, Oct 14, 2012 at 9:22 PM, Eric Firing > wrote: > > All, > > > > I think we are in a messy situation, and we need to reach some > agreement > > as to how to proceed. This has been discussed a bit in this thread: > > > > > http://sourceforge.net/mailarchive/forum.php?thread_name=507AFDC6.8000801%40hawaii.edu&forum_name=matplotlib-devel > > > > The name of that thread did not reflect the importance of the > discussion > > it prompted, hence the present message. > > > > To summarize my view: > > > > 1) We have a flood of PEP8 PRs based on master, many of which have > been > > merged, some by myself--so I have no objection to this aspect of the > > situation, though I would have preferred a slower pace, a garden hose > > rather than a fire hose. I am happy to see continued merging of > these > > PRs into master. > > > > 2) We are also trying to stabilize v1.2.x, getting in the last few > bug > > fixes and doc updates, so we can get a release out, with a high > > probability that it will be solid. > > > > 3) The potential disagreement is over whether the PEP8 changes > should be > > cherry-picked into v1.2.x, or simply left in master. I favor the > latter > > course. First, because massive code churn shortly before a release > > seems unwise. Second, because I think we should stick to the > strategy > > we started with, in which an effort is made to choose the most > > appropriate target for each PR, frequently merge the maintenance > branch >
[matplotlib-devel] request for code review: updates to Sankey class
Hello, I made a few minor changes to the Sankey class. They are listed at: https://github.com/kdavies4/matplotlib/compare/master...sankey5 Please review this and let me know if I can submit a pull request. Thanks. Kevin -- Don't let slow site performance ruin your business. Deploy New Relic APM Deploy New Relic app performance management and know exactly what is happening inside your Ruby, Python, PHP, Java, and .NET app Try New Relic at no cost today and get our sweet Data Nerd shirt too! http://p.sf.net/sfu/newrelic-dev2dev___ Matplotlib-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
Re: [matplotlib-devel] request for code review: updates to Sankey class
On Tue, Oct 16, 2012 at 2:55 PM, Kevin Davies wrote: > Hello, > > I made a few minor changes to the Sankey class. They are listed at: > https://github.com/kdavies4/matplotlib/compare/master...sankey5 > > Please review this and let me know if I can submit a pull request. > > Thanks. > > Kevin Thanks for taking the time to fix up a part of the codebase! If you make a pull request out of your code, we'll be able to leave inline comments on your patches. Nonetheless, I have some feedback for you after a (very) quick glance: 1) I don't think 'orientations' is a python keyword. What is the error you were getting? In any case, changing it breaks backwards compatibility so I'd be an advocate of keeping 'orientations'. Unless, of course, the error you were getting was serious. 2) Your changes appear to be, mainly, cosmetic. This is good but may cause issues with some of the PEP8 pull requests we have been getting. Have you rebased to make sure these changes are incorporated? 3) Inline with my PEP8 remark in 2) above. You have some lines (maybe only one or two) that look longer than 79 characters. Other than that, I think you should turn this into a pull request so you can get more feedback on an interactive level. Best, Damon -- Damon McDougall http://www.damon-is-a-geek.com B2.39 Mathematics Institute University of Warwick Coventry West Midlands CV4 7AL United Kingdom -- Don't let slow site performance ruin your business. Deploy New Relic APM Deploy New Relic app performance management and know exactly what is happening inside your Ruby, Python, PHP, Java, and .NET app Try New Relic at no cost today and get our sweet Data Nerd shirt too! http://p.sf.net/sfu/newrelic-dev2dev ___ Matplotlib-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
[matplotlib-devel] Color Parsing Bug
There seems to be an issue with how arguments are parsed when it comes
to determining the color of a line. Generally, it seems that 'c'
takes precedence over 'color'. However, this precedence seems to
change with the number of passed kwargs.
-
import matplotlib.pyplot as plt
import numpy as np
# 'c' seems to have precedence over 'color'
plt.plot(np.arange(10)-2, c='b', color='r') # line is blue
plt.plot(np.arange(10)-1, color='r', c='b') # line is blue
# But...
x = {'c': 'b',
'color': 'r',
'label': 'blah',
'linestyle': '-',
'linewidth': 3,
'marker': None}
# Some strange parsing error
plt.plot(np.arange(10)+1, **x) # line is red
del x['marker'] # delete any key but 'c' or 'color'
plt.plot(np.arange(10)+2, **x) # line is blue
x['zorder'] = 3 # add back any key
plt.plot(np.arange(10)+3, **x) # line is red
plt.show()
--
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct
___
Matplotlib-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
Re: [matplotlib-devel] request for code review: updates to Sankey class
Thanks for your comments! see below... On 10/16/2012 10:14 AM, Damon McDougall wrote: > On Tue, Oct 16, 2012 at 2:55 PM, Kevin Davies wrote: >> Hello, >> >> I made a few minor changes to the Sankey class. They are listed at: >> https://github.com/kdavies4/matplotlib/compare/master...sankey5 >> >> Please review this and let me know if I can submit a pull request. >> >> Thanks. >> >> Kevin > Thanks for taking the time to fix up a part of the codebase! > > If you make a pull request out of your code, we'll be able to leave > inline comments on your patches. Nonetheless, I have some feedback for > you after a (very) quick glance: I submitted the pull request. > 1) I don't think 'orientations' is a python keyword. What is the error > you were getting? In any case, changing it breaks backwards > compatibility so I'd be an advocate of keeping 'orientations'. Unless, > of course, the error you were getting was serious. The problem was on my end (In my code, I intercepted orientations as a named argument but assumed it being passed through **kwargs). I reverted to the original. Thanks for your patience. I'm sorry. > 2) Your changes appear to be, mainly, cosmetic. This is good but may > cause issues with some of the PEP8 pull requests we have been getting. > Have you rebased to make sure these changes are incorporated? I rebased off master after pulling from origin. That's correct, right? > 3) Inline with my PEP8 remark in 2) above. You have some lines (maybe > only one or two) that look longer than 79 characters. I re-wrapped everything to 79 characters. > Other than that, I think you should turn this into a pull request so > you can get more feedback on an interactive level. > > Best, > Damon > -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct ___ Matplotlib-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
