From: Damon M. <dam...@gm...> - 2012-09-20 21:30:32
|
Greetings denizens, I have been playing with custom triangulations in the plot_trisurf method of the mplot3d toolkit. I thought I would share my sweet creation: https://www.dropbox.com/s/ccptm6ok7nd3yn5/mobius.png Let me know your thoughts. I should probably make a pull request out of this, but the code is not currently readable by humans. I will tidy it up first. Make it look purrty. Lots of love, Damon -- Damon McDougall http://www.damon-is-a-geek.com B2.39 Mathematics Institute University of Warwick Coventry West Midlands CV4 7AL United Kingdom |
From: Ian T. <ian...@gm...> - 2012-09-21 07:27:13
|
On 20 September 2012 22:30, Damon McDougall <dam...@gm...>wrote: > I have been playing with custom triangulations in the plot_trisurf > method of the mplot3d toolkit. I thought I would share my sweet > creation: https://www.dropbox.com/s/ccptm6ok7nd3yn5/mobius.png > > Let me know your thoughts. I should probably make a pull request out > of this, but the code is not currently readable by humans. I will tidy > it up first. Make it look purrty. > Yes please! Ideally it would be good if all mplot3d functions supported the same arg and kwarg combinations as their 2d equivalents, for consistency. You may have done this already, but if not you might find the 2d tripcolor code helpful - it calls Triangulation.get_from_args_and_kwargs(*args, **kwargs) to do the initial heavy lifting. Forgive my cheek, but whilst you are looking at this area mplot3d.tricontour and tricontourf need similar improvement... A wider issue, and something I should have mentioned when you first implemented plot_trisurf, is that I don't like the function name. It seems unnecessary to have the plot_ at the front as most of the other plot-related functions manage without it. My unease extends to plot_surface and plot_wireframe as well. I guess we can't just change such names now that they are being used, but eventually I would like to see better naming within mplot3d. Thanks, Ian |
From: Damon M. <dam...@gm...> - 2012-09-21 10:43:51
|
On Fri, Sep 21, 2012 at 8:27 AM, Ian Thomas <ian...@gm...> wrote: > On 20 September 2012 22:30, Damon McDougall <dam...@gm...> > wrote: >> >> I have been playing with custom triangulations in the plot_trisurf >> method of the mplot3d toolkit. I thought I would share my sweet >> creation: https://www.dropbox.com/s/ccptm6ok7nd3yn5/mobius.png >> >> Let me know your thoughts. I should probably make a pull request out >> of this, but the code is not currently readable by humans. I will tidy >> it up first. Make it look purrty. > > > Yes please! Ideally it would be good if all mplot3d functions supported the > same arg and kwarg combinations as their 2d equivalents, for consistency. > You may have done this already, but if not you might find the 2d tripcolor > code helpful - it calls Triangulation.get_from_args_and_kwargs(*args, > **kwargs) to do the initial heavy lifting. > Yes, I had seen it. Thank you. Unfortunately, using this is non-trivial when you have an extra 'z' variable. There are a couple of options to get around this: 1. Re-write a 3d version and add it to matplotlib.tri. Pros of this approach are that the 3d methods would basically only require one extra line, a call to Triangulation.get_from_args_and_kwargs_3d(*args, **kwargs). Cons of this approach are code repetition. Also, I'm not a fan of putting 3d code into main mpl codebase. I think it should stay in the toolkit. 2. Implement option 1. but in the mplot3d toolkit rather than the main codebase. 3. Create a new args_2d tuple exactly equal to *args but with 'z' removed. Then rely on the 2d code with: Triangulation.get_from_args_and_kwargs(args_2d, **kwargs). Since this option would be common to all 3d functions relying on the 2d heavy lifting, it could be wrapped up into a convenience function. In my opinion, option 3. is preferable. Option 3. will also produce commits with the highest signal-to-noise ratio. > Forgive my cheek, but whilst you are looking at this area mplot3d.tricontour > and tricontourf need similar improvement... > Forgiven. I might as well do it whilst I'm already half-way down the rabbit hole. > A wider issue, and something I should have mentioned when you first > implemented plot_trisurf, is that I don't like the function name. It seems > unnecessary to have the plot_ at the front as most of the other plot-related > functions manage without it. My unease extends to plot_surface and > plot_wireframe as well. I guess we can't just change such names now that > they are being used, but eventually I would like to see better naming within > mplot3d. > Agreed. Not sure how best to solve this. Furthermore, I think it should be implemented in a pull request separate to the one migrating these 3d methods to custom triangulations. -- Damon McDougall http://www.damon-is-a-geek.com B2.39 Mathematics Institute University of Warwick Coventry West Midlands CV4 7AL United Kingdom |
From: Benjamin R. <ben...@ou...> - 2012-09-21 13:38:48
|
On Fri, Sep 21, 2012 at 6:43 AM, Damon McDougall <dam...@gm...>wrote: > On Fri, Sep 21, 2012 at 8:27 AM, Ian Thomas <ian...@gm...> wrote: > > On 20 September 2012 22:30, Damon McDougall <dam...@gm...> > > wrote: > >> > >> I have been playing with custom triangulations in the plot_trisurf > >> method of the mplot3d toolkit. I thought I would share my sweet > >> creation: https://www.dropbox.com/s/ccptm6ok7nd3yn5/mobius.png > >> > >> Let me know your thoughts. I should probably make a pull request out > >> of this, but the code is not currently readable by humans. I will tidy > >> it up first. Make it look purrty. > > > > > > Yes please! Ideally it would be good if all mplot3d functions supported > the > > same arg and kwarg combinations as their 2d equivalents, for consistency. > > You may have done this already, but if not you might find the 2d > tripcolor > > code helpful - it calls Triangulation.get_from_args_and_kwargs(*args, > > **kwargs) to do the initial heavy lifting. > > > > Yes, I had seen it. Thank you. Unfortunately, using this is > non-trivial when you have an extra 'z' variable. There are a couple of > options to get around this: > > 1. Re-write a 3d version and add it to matplotlib.tri. Pros of this > approach are that the 3d methods would basically only require one > extra line, a call to Triangulation.get_from_args_and_kwargs_3d(*args, > **kwargs). Cons of this approach are code repetition. Also, I'm not a > fan of putting 3d code into main mpl codebase. I think it should stay > in the toolkit. > > 2. Implement option 1. but in the mplot3d toolkit rather than the main > codebase. > > 3. Create a new args_2d tuple exactly equal to *args but with 'z' > removed. Then rely on the 2d code with: > Triangulation.get_from_args_and_kwargs(args_2d, **kwargs). Since this > option would be common to all 3d functions relying on the 2d heavy > lifting, it could be wrapped up into a convenience function. > > In my opinion, option 3. is preferable. Option 3. will also produce > commits with the highest signal-to-noise ratio. > > I agree with option 3. It fits in nicely with the paradigms of the existing codebase (my recent contributions excluded...) > > Forgive my cheek, but whilst you are looking at this area > mplot3d.tricontour > > and tricontourf need similar improvement... > > > > Forgiven. I might as well do it whilst I'm already half-way down the > rabbit hole. > > > A wider issue, and something I should have mentioned when you first > > implemented plot_trisurf, is that I don't like the function name. It > seems > > unnecessary to have the plot_ at the front as most of the other > plot-related > > functions manage without it. My unease extends to plot_surface and > > plot_wireframe as well. I guess we can't just change such names now that > > they are being used, but eventually I would like to see better naming > within > > mplot3d. > > > > Agreed. Not sure how best to solve this. Furthermore, I think it > should be implemented in a pull request separate to the one migrating > these 3d methods to custom triangulations. > > I wouldn't be against going down a deprecation path for renaming these functions, but for what gains? The names have been there since before I took up this toolkit, and ultimately, these functions aren't typed so often that brevity would gain one much. Definitely any new functions should not take up that naming, but I see no real compelling reason to change the existing names. Cheers! Ben Root |
From: Damon M. <dam...@gm...> - 2012-09-22 09:43:14
|
On Fri, Sep 21, 2012 at 2:38 PM, Benjamin Root <ben...@ou...> wrote: > I wouldn't be against going down a deprecation path for renaming these > functions, but for what gains? The names have been there since before I > took up this toolkit, and ultimately, these functions aren't typed so often > that brevity would gain one much. Definitely any new functions should not > take up that naming, but I see no real compelling reason to change the > existing names. Technically, 1.2 hasn't been released yet. We can still change plot_trisurf to trisurf, though it appears there is still uncertainty regarding whether there will actually be a third release candidate. -- Damon McDougall http://www.damon-is-a-geek.com B2.39 Mathematics Institute University of Warwick Coventry West Midlands CV4 7AL United Kingdom |
From: Ian T. <ian...@gm...> - 2012-09-21 17:22:01
|
On 21 September 2012 14:38, Benjamin Root <ben...@ou...> wrote: > On Fri, Sep 21, 2012 at 6:43 AM, Damon McDougall < > dam...@gm...> wrote: > >> On Fri, Sep 21, 2012 at 8:27 AM, Ian Thomas <ian...@gm...> >> wrote: >> > On 20 September 2012 22:30, Damon McDougall <dam...@gm...> >> > wrote: >> > 3. Create a new args_2d tuple exactly equal to *args but with 'z' >> removed. Then rely on the 2d code with: >> Triangulation.get_from_args_and_kwargs(args_2d, **kwargs). Since this >> option would be common to all 3d functions relying on the 2d heavy >> lifting, it could be wrapped up into a convenience function. >> >> In my opinion, option 3. is preferable. Option 3. will also produce >> commits with the highest signal-to-noise ratio. >> >> > I agree with option 3. It fits in nicely with the paradigms of the > existing codebase (my recent contributions excluded...) > I am happy with option 3 too, but I don't think you need to do as much as this. The existing 2d triplot/tripcolor/tricontour call Triangulation.get_from_args_and_kwargs, which removes the various args/kwargs needed for the triangulation and leaves the remainder for the calling function to process. For example lib/matplotlib/tri/tricontour.py lines 86 to 88: tri, args, kwargs = \ Triangulation.get_from_args_and_kwargs(*args, **kwargs) z = np.asarray(args[0]) Can't you just do the same here, or am I missing something? > A wider issue, and something I should have mentioned when you first >> > implemented plot_trisurf, is that I don't like the function name. It >> seems >> > unnecessary to have the plot_ at the front as most of the other >> plot-related >> > functions manage without it. My unease extends to plot_surface and >> > plot_wireframe as well. I guess we can't just change such names now >> that >> > they are being used, but eventually I would like to see better naming >> within >> > mplot3d. >> > >> >> >> Agreed. Not sure how best to solve this. Furthermore, I think it >> should be implemented in a pull request separate to the one migrating >> these 3d methods to custom triangulations. >> >> > I wouldn't be against going down a deprecation path for renaming these > functions, but for what gains? The names have been there since before I > took up this toolkit, and ultimately, these functions aren't typed so often > that brevity would gain one much. Definitely any new functions should not > take up that naming, but I see no real compelling reason to change the > existing names. > The gain is that of more consistent naming of the mplot3d api functions. The argument that we should keep names because they have been around for a long time is not a strong one if the names are poor choices in the first place. But I am not particularly concerned as I think that mplot3d has some bigger problems to solve than this e.g. correct rendering of nested polygons, z-order rendering, so maybe I should keep quiet about such details! Ian |
From: Damon M. <dam...@gm...> - 2012-09-22 09:40:19
|
On Fri, Sep 21, 2012 at 6:21 PM, Ian Thomas <ian...@gm...> wrote: > I am happy with option 3 too, but I don't think you need to do as much as > this. The existing 2d triplot/tripcolor/tricontour call > Triangulation.get_from_args_and_kwargs, which removes the various > args/kwargs needed for the triangulation and leaves the remainder for the > calling function to process. For example lib/matplotlib/tri/tricontour.py > lines 86 to 88: > > tri, args, kwargs = \ > Triangulation.get_from_args_and_kwargs(*args, **kwargs) > z = np.asarray(args[0]) > > Can't you just do the same here, or am I missing something? Ah ok, I see. I was assuming a plot_trisurf(x, y, z, triangles, ...) signature. Copying the tricontour signature would be better for consistency reasons. It appears that I was the one missing something! Thanks for that. -- Damon McDougall http://www.damon-is-a-geek.com B2.39 Mathematics Institute University of Warwick Coventry West Midlands CV4 7AL United Kingdom |
From: Damon M. <dam...@gm...> - 2012-09-22 11:06:13
|
On Sat, Sep 22, 2012 at 10:40 AM, Damon McDougall <dam...@gm...> wrote: > Ah ok, I see. I was assuming a plot_trisurf(x, y, z, triangles, ...) > signature. Copying the tricontour signature would be better for > consistency reasons. It appears that I was the one missing something! I just realised I'm not making any sense here. Please disregard most of what I say. I'm using the get_from_args_and_kwargs method and everything is fine. -- Damon McDougall http://www.damon-is-a-geek.com B2.39 Mathematics Institute University of Warwick Coventry West Midlands CV4 7AL United Kingdom |