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

need to update docstring in netcdf_output_writer.jl #2986

Closed
francispoulin opened this issue Mar 18, 2023 · 5 comments
Closed

need to update docstring in netcdf_output_writer.jl #2986

francispoulin opened this issue Mar 18, 2023 · 5 comments

Comments

@francispoulin
Copy link
Collaborator

I tried running two examples in the docstring of netcdf_output_writer.jl and found something that I believe needs to be updated.

Line 274, defines the nodes but this didn't work for me. I believe we should change it to the following:

xC, yF = xnodes(Center, grid), ynodes(Face, grid)

Also, it might helpful to add this line at the beginning, as otherwise, the example does not run,

using Oceananigans.Grids: xnodes, ynodes, znodes

If people agree with this I can make the changes but thought I would suggest it before creating a PR.

@navidcy
Copy link
Collaborator

navidcy commented Mar 19, 2023

The docstring is a doctest so how can it not work..?

Are you sure you are on the same version of Oceananigans as the docs you are pointing out?

@navidcy
Copy link
Collaborator

navidcy commented Mar 19, 2023

xnodes, ynodes, znodes are exported so users should have access to them after saying using Oceananigans.

@navidcy
Copy link
Collaborator

navidcy commented Mar 19, 2023

on main:

julia> using Oceananigans
[ Info: Precompiling Oceananigans [9e8cae18-63c1-5223-a75c-80ca9d6e9a09]
[ Info: Oceananigans will use 6 threads

julia> xnodes
xnodes (generic function with 15 methods)

julia>

help?> xnodes
search: xnodes

  xnodes(grid, LX, LY, LZ, with_halos=false)


  Return the positions over the interior nodes on grid in the x-direction for the location LX, LY, LZ. For Bounded directions, Face nodes
  include the boundary points.

  See znodes for examples.

I believe you are not reading the docs that correspond to the version you are on?

@glwagner
Copy link
Member

@francispoulin note if that you just paste the raw link, github will show the source code (a bit easier to read):

xC, yF = xnodes(grid, Center()), ynodes(grid, Face())

The above is the new syntax

@francispoulin
Copy link
Collaborator Author

Sorry for my mistake @navidcy and @glwagner, and thanks for the clarification.

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

No branches or pull requests

3 participants