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

Updates in docstrings for predefined dynamical systems #113

Merged
merged 11 commits into from
Jun 19, 2021

Conversation

navidcy
Copy link
Contributor

@navidcy navidcy commented Jun 18, 2021

This PR updates some of the docstrings for predefined dynamical systems.

@navidcy
Copy link
Contributor Author

navidcy commented Jun 18, 2021

Is there a way to build the docs locally so I can test before I finalize the PR?

cc: @Datseris

@navidcy navidcy changed the title [WIP] Update docstrings Update docstrings Jun 18, 2021
@Datseris
Copy link
Member

Thanks,

Is there a way to build the docs locally so I can test before I finalize the PR?

You'd have to run DynamicalSystems/docs/make.jl for the main DynamicalSystems.jl package, with this package checked out at your PR. Alternatively, if you use Atom/VSCode/Jupyter, the docstrings there can be rendered with markdown. So use the package there and search its docstrings.

P.s.: Not sure if unicode like ω will be rendered in latex, we have to check.

p.s.s.: Please also increment patch version in Project.toml.

@navidcy
Copy link
Contributor Author

navidcy commented Jun 18, 2021

You'd have to run DynamicalSystems/docs/make.jl for the main DynamicalSystems.jl package, with this package checked out at your PR. Alternatively, if you use Atom/VSCode/Jupyter, the docstrings there can be rendered with markdown. So use the package there and search its docstrings.

Deal

P.s.: Not sure if unicode like ω will be rendered in latex, we have to check.

Unicode will compile in latex. See https://github.com/FourierFlows/GeophysicalFlows.jl/blob/bb88f49db31b7eb170c14e515a0364191894fcac/src/twodnavierstokes.jl#L457-L466 and https://fourierflows.github.io/GeophysicalFlowsDocumentation/dev/lib/functions/#GeophysicalFlows.TwoDNavierStokes.energy_work

p.s.s.: Please also increment patch version in Project.toml.

ok!

@navidcy
Copy link
Contributor Author

navidcy commented Jun 18, 2021

any ideas?

(base) DynamicalSystems.jl/ (master) $ julia --project=docs/ -e 'using Pkg; Pkg.develop(PackageSpec(path=pwd())); Pkg.instantiate()'; julia --project=docs docs/make.jl; open docs/build/index.html
Path `/Users/navid/Research/DynamicalSystems.jl` exists and looks like the correct package. Using existing path.
   Resolving package versions...
  No Changes to `~/Research/DynamicalSystems.jl/docs/Project.toml`
  No Changes to `~/Research/DynamicalSystems.jl/docs/Manifest.toml`
  Activating environment at `~/Research/DynamicalSystems.jl/docs/Project.toml`
INTEL MKL ERROR: dlopen(/Users/navid/.julia/conda/3/lib/libmkl_intel_thread.dylib, 9): Library not loaded: @rpath/libiomp5.dylib
  Referenced from: /Users/navid/.julia/conda/3/lib/libmkl_intel_thread.dylib
  Reason: image not found.
Intel MKL FATAL ERROR: Cannot load libmkl_intel_thread.dylib.

@Datseris
Copy link
Member

No, not really. I can't imagine an MKL error being related with DynamicalSystems.jl. Can check the docstrings over the weekend and merge though!

@navidcy
Copy link
Contributor Author

navidcy commented Jun 18, 2021

It's PyPlot-related. I'm trying to figure it out.

$ julia --project=docs/                                                                              [5:47:54]
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.6.1 (2021-04-23)
 _/ |\__'_|_|_|\__'_|  |
|__/                   |

julia> using PyPlot
INTEL MKL ERROR: dlopen(/Users/navid/.julia/conda/3/lib/libmkl_intel_thread.dylib, 9): Library not loaded: @rpath/libiomp5.dylib
  Referenced from: /Users/navid/.julia/conda/3/lib/libmkl_intel_thread.dylib
  Reason: image not found.
Intel MKL FATAL ERROR: Cannot load libmkl_intel_thread.dylib.

@navidcy
Copy link
Contributor Author

navidcy commented Jun 18, 2021

This suggestion did the trick.

@navidcy
Copy link
Contributor Author

navidcy commented Jun 18, 2021

I was tempted to change kwarg G to g but I thought in the end that this is a different PR. But is there a "convention" for using capital letters for parameters?

@navidcy
Copy link
Contributor Author

navidcy commented Jun 18, 2021

Seems ok

Screen Shot 2021-06-19 at 6 01 33 am

I may tweak it a bit :)

@navidcy
Copy link
Contributor Author

navidcy commented Jun 18, 2021

OK one question: I used unicode subscripts because they render in REPL's help mode. But in LaTeX they do render a bit differently compared to using, e.g., _1. Shall I leave it as is or homogenize notation with rest of docstrings by using conventional latex notation?

@navidcy
Copy link
Contributor Author

navidcy commented Jun 18, 2021

OK, I'm done here. Let me know what you think or if you'd like me to modify anything. If you are happy with everything feel free to merge.

@navidcy navidcy changed the title Update docstrings Updates in docstrings for predefined dynamical systems Jun 18, 2021
@Datseris Datseris merged commit 6c57887 into JuliaDynamics:master Jun 19, 2021
@Datseris
Copy link
Member

thanks a lot @navidcy !

Minor comment: In the future, if you want to make many minor changes, it is actually easier to review and keep track if you put them all in one PR and add a bullet point list with the changes!

@navidcy navidcy deleted the docstrings-patch branch June 19, 2021 08:15
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