Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#11208 closed defect (fixed)

Remove numpy warnings in slope field

Reported by: kcrisman Owned by: jason, was
Priority: major Milestone: sage-5.0
Component: graphics Keywords:
Cc: jason, novoselt Merged in: sage-5.0.beta9
Authors: Douglas McNeil Reviewers: David Loeffler, Karl-Dieter Crisman
Report Upstream: Reported upstream. Little or no feedback. Work issues:
Branch: Commit:
Dependencies: #10489 Stopgaps:

Description

Having no arrows in an arrow plot (i.e., vector field that is a slope field) now causes warnings from matplotlib.

y = var('y') 
g = 1 
P=plot_slope_field(g,(x,3,4),(y,-1,1)) 
P 
<two sets of warnings> 

DSM has a diagnosis at this sage-support thread

P=plot_slope_field(g,(x,3,4),(y,-1,1),headlength=1e-8) 
works for me.  FYI, it's the following few lines in Quiver._h_arrows at fault: 
        minsh = self.minshaft * self.headlength 
        [....] 
        shrink = length/minsh 
        X0 = shrink * X0[np.newaxis,:] 
        Y0 = shrink * Y0[np.newaxis,:] 
Probably we should change the defaults and/or (if it's not done 
already) ask our matplotlib friends to special-case 0 for no 
arrowheads.

So maybe this should be reported upstream? See also #2922, which is also about the mpl quivers.

Attachments (2)

trac_11208_avoid_headless_quiver_warning.patch (1.5 KB) - added by dsm 8 years ago.
headlength = epsilon hack
trac_11208_avoid_headless_quiver_warning_v2.patch (1.5 KB) - added by dsm 8 years ago.
headlength = epsilon hack, rebased to 5.0.beta7

Download all attachments as: .zip

Change History (16)

comment:1 Changed 9 years ago by jason

  • Cc jason added
  • Milestone set to sage-4.7
  • Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.

This should be reported upstream, right?

comment:2 Changed 9 years ago by kcrisman

Probably, and I say so on sage-support, but I don't know how to make this pure matplotlib, and maybe they would say it's user error? But you are almost certainly right.

comment:3 Changed 9 years ago by kcrisman

I don't think this was ever reported upstream. And now it's going to cause a problem for our PREP calculus tutorial... grumble, grumble.

comment:4 Changed 9 years ago by mhampton

For the moment it seems OK to use Doug McNeil?'s hack of setting: headlength=1e-8 Its better than leaving this problem around any longer in my opinion.

comment:5 Changed 8 years ago by novoselt

  • Cc novoselt added

comment:6 Changed 8 years ago by dsm

  • Status changed from new to needs_review

Patch attached. Wasn't sure of the idiom for doctesting the warnings; went with what I would have done in a non-Sage project.

Changed 8 years ago by dsm

headlength = epsilon hack

comment:7 Changed 8 years ago by davidloeffler

  • Dependencies set to #10489
  • Reviewers set to PatchBot
  • Status changed from needs_review to needs_work

This doesn't apply to the current Sage dev version -- it seems to conflict with #10489. Can you rebase it?

comment:8 Changed 8 years ago by dsm

  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. Little or no feedback.

comment:9 Changed 8 years ago by kcrisman

  • Authors set to Douglas McNeil
  • Reviewers changed from PatchBot to David Loeffler, Karl-Dieter Crisman

On the plus side, it works! DSM, just be sure to not replace the previous patch so that we can compare the two, but the rebase should be nearly trivial (I almost did it just now, but decided you were probably in the midst). The only thing I could possibly say is that I would put the "(trac #11208)::" on the next line, but it's not really that important.

So positive review, modulo rebase.

Changed 8 years ago by dsm

headlength = epsilon hack, rebased to 5.0.beta7

comment:10 Changed 8 years ago by dsm

  • Status changed from needs_work to needs_review

Dropped a message on matplotlib-devel; at least one dev thought special-casing zero was a good idea, so I'll work up a matplotlib patch if I get some time. Not much pressure to do it on our end now, though. [PS @kcrisman -- not only was I in the midst, I was typing things like 'hg export tip' as you were writing that..]

comment:11 Changed 8 years ago by kcrisman

  • Priority changed from minor to major
  • Status changed from needs_review to positive_review

Great, make sure to post a link to the discussion on matplotlib-devel. This applies to beta7 and fixes something we REALLY should have fixed a long time ago.

By the way, it turns out we now have this nice :trac: thing for documentation... not needed here, but just fyi for next time, I just found out about five minutes ago.

comment:12 Changed 8 years ago by davidloeffler

Apply trac_11208_avoid_headless_quiver_warning_v2.patch

(for patchbot)

comment:13 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.0.beta9
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:14 Changed 8 years ago by kcrisman

Upstream report is here.

Note: See TracTickets for help on using tickets.