#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)
Change History (16)
comment:1 Changed 9 years ago by
- Cc jason added
- Milestone set to sage-4.7
- Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.
comment:2 Changed 9 years ago by
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
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
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
- Cc novoselt added
comment:6 Changed 8 years ago by
- 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.
comment:7 Changed 8 years ago by
- 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
- 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
- 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.
comment:10 Changed 8 years ago by
- 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
- 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
Apply trac_11208_avoid_headless_quiver_warning_v2.patch
(for patchbot)
comment:13 Changed 8 years ago by
- 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
Upstream report is here.
This should be reported upstream, right?