Ticket #11208 (closed defect: fixed)

Opened 2 years ago

Last modified 14 months ago

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 Work issues:
Report Upstream: Reported upstream. Little or no feedback. Reviewers: David Loeffler, Karl-Dieter Crisman
Authors: Douglas McNeil Merged in: sage-5.0.beta9
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

trac_11208_avoid_headless_quiver_warning.patch Download (1.5 KB) - added by dsm 15 months ago.
headlength = epsilon hack
trac_11208_avoid_headless_quiver_warning_v2.patch Download (1.5 KB) - added by dsm 14 months ago.
headlength = epsilon hack, rebased to 5.0.beta7

Change History

comment:1 Changed 2 years ago by jason

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

This should be reported upstream, right?

comment:2 Changed 2 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 2 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 21 months 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 17 months ago by novoselt

  • Cc novoselt added

comment:6 Changed 15 months 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 15 months ago by dsm

headlength = epsilon hack

comment:7 Changed 14 months ago by davidloeffler

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

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

comment:8 Changed 14 months ago by dsm

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

comment:9 Changed 14 months ago by kcrisman

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

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 14 months ago by dsm

headlength = epsilon hack, rebased to 5.0.beta7

comment:10 Changed 14 months 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 14 months 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 14 months ago by davidloeffler

Apply trac_11208_avoid_headless_quiver_warning_v2.patch

(for patchbot)

comment:13 Changed 14 months ago by jdemeyer

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

comment:14 Changed 14 months ago by kcrisman

Upstream report is  here.

Note: See TracTickets for help on using tickets.