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

Nuke BasicModel legacy constructor and fix `NonDimensionalMode… #496

Merged
merged 4 commits into from
Oct 23, 2019

Conversation

ali-ramadhan
Copy link
Member

@ali-ramadhan ali-ramadhan commented Oct 23, 2019

This PR nukes the BasicModel legacy constructor to avoid potential confusion.

I also made the NonDimensionalModel constructor act more like the Model constructor. Also fixed a typo there and added a test.

Resolves #429

@ali-ramadhan ali-ramadhan added the cleanup 🧹 Paying off technical debt label Oct 23, 2019
@ali-ramadhan ali-ramadhan added this to the JOSS milestone Oct 23, 2019
@codecov
Copy link

codecov bot commented Oct 23, 2019

Codecov Report

Merging #496 into master will increase coverage by 0.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #496      +/-   ##
==========================================
+ Coverage   73.31%   73.46%   +0.15%     
==========================================
  Files          27       27              
  Lines        1525     1515      -10     
==========================================
- Hits         1118     1113       -5     
+ Misses        407      402       -5
Impacted Files Coverage Δ
src/Oceananigans.jl 62.5% <ø> (ø) ⬆️
src/diagnostics.jl 79.31% <ø> (ø) ⬆️
src/models.jl 88.23% <ø> (+8.68%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f3ef52...2373483. Read the comment docs.

@ali-ramadhan ali-ramadhan changed the title Nuke BasicModel legacy constructor and fix NonDimensionalModel constructor Nuke BasicModel legacy constructor and fix `NonDimensionalMode… Oct 23, 2019
@ali-ramadhan ali-ramadhan merged commit 27493f3 into master Oct 23, 2019
@ali-ramadhan ali-ramadhan deleted the ar/better-model-constructors branch October 23, 2019 18:39
arcavaliere pushed a commit to arcavaliere/Oceananigans.jl that referenced this pull request Nov 6, 2019
…iMA#496)

Nuke `BasicModel` legacy constructor and fix `NonDimensionalModel` constructor

Former-commit-id: 27493f3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup 🧹 Paying off technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up legacy constructors
3 participants