Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding plot numticks option to plot(m,'b') #165

Merged
merged 9 commits into from
Jan 2, 2021
Merged

Conversation

krober10nd
Copy link
Collaborator

  • ... for bathymetry plot type

@krober10nd krober10nd changed the title adding plot ticks adding plot numticks option to plot(m,'b') Dec 23, 2020
@WPringle
Copy link
Collaborator

WPringle commented Dec 23, 2020

Do this work correctly? Should be numticks(1) -1 or numticks(1)?
Maybe show a before/after image would be helpful.

@krober10nd
Copy link
Collaborator Author

why would it be numticks -1 ?

@WPringle
Copy link
Collaborator

because the logaxis one is like that. if your ticks are 0-10 then you have 11 ticks but 10 colors.

@krober10nd
Copy link
Collaborator Author

Yea, I think then numticks should be called number of intervals (num_intervals)?

@WPringle
Copy link
Collaborator

Probably it is more intuitive to change to num_intervals yes. Then make the number of numticks = num_intervals + 1 in the code.

- adding a plotter subfuction used by a few of the case selects to streamline things
- changing numticks option to 'colormap' to specify the colormap intervals
- removing the mannings, cfval, ifric, tau0 fort13 options to use the arbitrary f13 attribute plotter instead
- adding help for transect and sponge options
…ad of directory filename parts (one can add the directory of the files to path if necessary)
Copy link
Collaborator

@WPringle WPringle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made edits myself to plot

@krober10nd
Copy link
Collaborator Author

Hm...while we have this door open to improve the plotting api, I'd like to make a few suggestions:

  1. The proj, 1 switch is awkward. Can't we just detect if a projection is used from the strcmp? i.e.,
plot(m,'tri','lamb');

vs.

plot(m,'tri',1,'lamb');
  1. The subsetting optional should be optional. I.e., I shouldn't have to enter a blank array to get to the kwarg args.
plot(m,'tri','lamb',[],'fontsize',16); 

should be

plot(m,'b',1,'lamb','fontsize',16);
  1. Reading in from a msh file results in an empty projection and leads to a fault if the user does not specify it. I forget why did we get rid of the default geographic projection?

@krober10nd
Copy link
Collaborator Author

also, as always, I suggest that we modify the examples to reflect these plotting capabilities. The examples are what everyone looks at it.

@krober10nd
Copy link
Collaborator Author

Okay, following up on my suggestions I made the API to msh.plot much simpler hopefully.

Now everything is specified as name/value pairs so you don't need to remember the order or what not

  • For instance this plots the triangulation assuming the msh object has the projection information
m = plot(m)
  • This plots the bathymetry with a logarithmic plot using a Lambert conformal projection
m = plot(m,'type','blog','proj','lamb');
  • This plots a subsection of the triangulation
m = plot(m,'type','tri','proj','lamb','bou',[-74, -70; 38 40]);

and so on.

I would suggest to change the kwarg bou to subset.

@WPringle
Copy link
Collaborator

Yeah I was thinking that same thing with those sorts of changes. So now we just need to modify the examples I guess?

@krober10nd
Copy link
Collaborator Author

Yeah I was thinking that same thing with those sorts of changes. So now we just need to modify the examples I guess?

Yep

@krober10nd
Copy link
Collaborator Author

this looks like a good improvement. Are we ready to merge this in?

@WPringle
Copy link
Collaborator

WPringle commented Jan 1, 2021

Just modified the examples so I think we are ready now. Can you just test one on your end maybe?

@krober10nd
Copy link
Collaborator Author

krober10nd commented Jan 1, 2021

Okay, ran the majority of examples and didn't encounter any problems. However, for example Example_ECGC is not interpolating the bathymetry to the vertices correctly. I'm getting a result that shows the shelf is overland and past the shelf is underwater.

Maybe somethings wrong with my SRTM download? The JBAY example appears to interpolate just fine.

@krober10nd
Copy link
Collaborator Author

The GBAY example with the floodplain also works fine.

@krober10nd
Copy link
Collaborator Author

Okay, it was a mistake on my part. I've tested all examples and they ran to completion and produced reasonable plots.

@krober10nd krober10nd merged commit 51ed9f3 into Projection Jan 2, 2021
@krober10nd krober10nd deleted the add_ticks_bathy branch January 2, 2021 19:45
Jiangchao3 pushed a commit to Jiangchao3/OceanMesh2D that referenced this pull request Jan 3, 2021
adding plot numticks option to `plot(m,'b')` (CHLNDDEV#165)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants