Mesh tutorials #157
Mesh tutorials #157
Conversation
| if showIt: | ||
| plt.show() | ||
|
|
||
| return fig |
There was a problem hiding this comment.
We might want to think about the return fig. It has the fig parameter in the call, so you can provide an exiting figure, in which case you don't want/need it to be returned. Is it always required?
But anyhow, as long as it is streamlined with discretize then I'm fine with it.
There was a problem hiding this comment.
The main motivation for the return here would be if someone didn't provide it as an input, but instead let the figure handle be created by the function. Then if you want to add labels and such after-the-fact, you can access the figure handle
There was a problem hiding this comment.
Yeah, I think this is used a lot in modifying figures after the fact. Returning the thing you created seems like good practice.
| @@ -737,7 +737,7 @@ def plotGrid( | |||
| def plot_3d_slicer(self, v, xslice=None, yslice=None, zslice=None, | |||
| vType='CC', view='real', axis='xy', transparent=None, | |||
| clim=None, aspect='auto', grid=[2, 2, 1], | |||
| pcolorOpts=None, fig=None): | |||
| pcolorOpts=None, fig=None, showIt=False): | |||
There was a problem hiding this comment.
I probably would prefer showIt=True as default. Otherwise I have to provide showIt=True every time I call the slicer and I want to see it right away? Or am I missing something?
There was a problem hiding this comment.
I think the original motivation for the showIt was really for testing (cc @rowanc1). We didn't have the agg backend set on travis, so showing the plots made a mess. At this point, I think we can leave it out, or at the very least set the default as True
|
Thanks @dcowan. I just made two comments in the slicer class. I think you're changes will make it indeed more aligned with other functions in I never fully understood the logic behind the Anyhow, my two comments are just thoughts/ideas. I am fine with anything that is streamlined with the rest of discretize. |
Codecov Report
@@ Coverage Diff @@
## master #157 +/- ##
==========================================
+ Coverage 81.88% 83.76% +1.87%
==========================================
Files 22 22
Lines 4754 4756 +2
==========================================
+ Hits 3893 3984 +91
+ Misses 861 772 -89
Continue to review full report at Codecov.
|
|
The |
There was a problem hiding this comment.
Thanks for getting this started @dccowan! In addition to the comments from @prisae, I have a couple big picture comments before I dive into the code:
- can we please stick with the directory name
examplesinstead ofdemo_examplesfor where the examples live. - similarly for tutorials, lets stick with
tutorialsrather thantutorial_examples(it looks like in the conf.py bothtutorialsandtutorial_examplescurrently exist). For simplicity, I think we should stick with onlyexamplesandtutorials- do you have a strong motivation otherwise? - can you please remove the `demo_examples/Chile_GRAV_4_Miller.tar.gz, demo_examples/Chile_GRAV_4_Miller/... files? They were originally in the gitignore under examples, but when you renamed the examples directory it looks like they got added.
There was a problem hiding this comment.
When you renamed things, a bunch of extra files got added to this pr. I will go through in a bit more detail once we get that sorted. If you rename back to tutorials and examples, then the .gitignore should clean some of this up. It also looks like your machine adds .md5 files - I am not sure what these are, but you can add *.md5 to the gitignore to prevent these being added in the future.
| 'gallery_dirs': ['gallery', 'tutorials'], | ||
| 'examples_dirs': [ | ||
| '../demo_examples', | ||
| '../tutorial_examples/mesh_generation', |
There was a problem hiding this comment.
lets just stick with examples and tutorials for simplicity
| @@ -0,0 +1,278 @@ | |||
| :orphan: | |||
|
|
|||
There was a problem hiding this comment.
I believe this file is auto-generated, so we don't need to include it in the repository
| @@ -0,0 +1 @@ | |||
| e537c6487971da1f09c98da2b98bb670 | |||
There was a problem hiding this comment.
not sure what this is... might be a system file? could you please remove it?
| @@ -0,0 +1,74 @@ | |||
| .. note:: | |||
There was a problem hiding this comment.
is this auto-generated? if so, then we don't need to include it in the repository
| # (dt*A*d2fdphi2 - I - dt*A*L) * phi_ = (dt*A*d2fdphi2 - I)*phi - dt*A*dfdphi | ||
|
|
||
| h = [(0.25, n)] | ||
| M = discretize.TensorMesh([h, h]) |
There was a problem hiding this comment.
I realize this is probably copy-over, but lets try and stick with mesh instead of M for mesh objects in the examples
|
👋 Hi @dccowan, I see you closed this pr - do you have another working branch in progress? |
This PR is meant to create a section for tutorials. We started with tutorials for generating
There were also a couple of bug fixes. One in the surface-based refinement for QuadTree meshes. And another in the 'plot_3d_slicer' method.