-
Notifications
You must be signed in to change notification settings - Fork 238
Submodules for grids, solvers, diagnostics, and output writers #512
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard to wrap my head around this but scanning the new source structure, it looks like a move in the right direction.
Looks like there are some issues with examples, and that's why the doc builds failed. |
As a side note: I think it's ok to include "shared utils" at the top level within a submodule prior to importing logically-distinct functionality contained in separate files. Sometimes this can improve code-readability if there are a small number of utils. The need for a separate "utils.jl" file should decrease when the code structure is more modular and separated into logical subunits, I think. We can also have single-file submodules, as in Documenter.jl (I think their file structure looks very sane and manageable): https://github.com/JuliaDocs/Documenter.jl/tree/master/src There's still a bit of work we need to do to understand inter-submodule dependencies; once that's sorted out I think the top-level Oceananigans.jl file will clean up. |
By the way, with a |
Codecov Report
@@ Coverage Diff @@
## master #512 +/- ##
==========================================
- Coverage 67.68% 65.66% -2.02%
==========================================
Files 49 65 +16
Lines 1838 1896 +58
==========================================
+ Hits 1244 1245 +1
- Misses 594 651 +57
Continue to review full report at Codecov.
|
Hmmm, I like having separate Been thinking that the main At some point we should discuss how to structure the package code then everything should become much cleaner. |
This PR refactors each of
grids.jl
,poisson_solvers.jl
,diagnostics.jl
andoutput_writers.jl
into their own submodules.Moving stuff into modules is a pretty disruptive change that introduces tough merge conflicts into other PRs so I also created a Grids and a Solvers submodule in preparation for the vertically stretched grid as I didn't want to include them with vertically stretched PRs that could sit open for days.
The whole code still feels a bit messy to me but I think working on #497 will help a lot.
Helps with #456 and #495