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.

Sign up for GitHub

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

Jump to bottom

Test configs #17

Merged
merged 9 commits into from Jun 14, 2021
Merged

Test configs #17

merged 9 commits into from Jun 14, 2021

Conversation

jdha
Copy link
Contributor

@jdha jdha commented Jun 9, 2021 •
edited

  • Closes Add ideal test domains #10
  • Passes pre-commit run --all-files
  • Changes are documented in whats-new.rst
  • Project, label, and assignee tabs are populated

@jdha jdha self-assigned this Jun 9, 2021
@jdha jdha added this to In progress in domzgr.F90 -> pyDOMCFG via automation Jun 9, 2021
@jdha jdha added enhancement New feature or request task do something! labels Jun 9, 2021
@codecov
Copy link

codecov bot commented Jun 9, 2021 •
edited

Codecov Report

Merging #17 (eb5c1b2) into main (1edcec7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #17   +/-   ##
=======================================
  Coverage   93.12%   93.12%           
=======================================
  Files           4        4           
  Lines         160      160           
=======================================
  Hits          149      149           
  Misses         11       11
Flag Coverage Δ
unittests 93.12% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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 1edcec7...eb5c1b2. Read the comment docs.

@jdha jdha linked an issue Jun 9, 2021 that may be closed by this pull request
Add ideal test domains #10
Closed
malmans2
malmans2 reviewed Jun 9, 2021
View reviewed changes
pydomcfg/tests/bathymetry.py Outdated Show resolved Hide resolved
pydomcfg/tests/bathymetry.py Outdated Show resolved Hide resolved
pydomcfg/tests/bathymetry.py Outdated Show resolved Hide resolved
@jdha
Copy link
Contributor Author

jdha commented Jun 14, 2021 •
edited

test_configs ready for merge. whats-new.rst created the block on merging, so I've resolved that - but pre-commit is still indicating error during mergeable check - not too sure what I've done wrong here, please educate me! Perhaps one of you has to approve my Merge branch 'main' into test_configs action?

@malmans2
Copy link
Member

pre-commit.ci run

1 similar comment
@jdha
Copy link
Contributor Author

jdha commented Jun 14, 2021

pre-commit.ci run

domzgr.F90 -> pyDOMCFG automation moved this from In progress to Reviewer approved Jun 14, 2021
malmans2
malmans2 approved these changes Jun 14, 2021
View reviewed changes
Copy link
Member

@malmans2 malmans2 left a comment

Choose a reason for hiding this comment

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

LGTM!

pydomcfg/tests/bathymetry.py
Comment on lines +73 to +84
# Define sea mount bathymetry
ds["Bathymetry"] = depth * (
1.0
- 0.9
* np.exp(
-(
stiff
/ 40.0e3 ** 2
* ((ds.glamt - glamt_mid) ** 2 + (ds.gphit - gphit_mid) ** 2)
)
)
)
Copy link
Member

Choose a reason for hiding this comment

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

This is kind of hard to read... Maybe define a couple of variables? E.g. ((ds.glamt - glamt_mid) ** 2 + (ds.gphit - gphit_mid) ** 2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok - will fold it in with the DOME PR. It looked fine to me until BLACK reformatted it ;)

@jdha jdha merged commit fe0ac9e into main Jun 14, 2021
domzgr.F90 -> pyDOMCFG automation moved this from Reviewer approved to Done Jun 14, 2021
@jdha jdha deleted the test_configs branch June 14, 2021 08:33
@malmans2 malmans2 mentioned this pull request Jun 25, 2021
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@malmans2 malmans2 malmans2 approved these changes

Assignees

@jdha jdha

Labels
enhancement New feature or request task do something!
Projects
domzgr.F90 -> pyDOMCFG
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add ideal test domains
2 participants
@jdha @malmans2