-
Notifications
You must be signed in to change notification settings - Fork 13
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
Enable linting in physics #48
Conversation
.pre-commit-config.yaml
Outdated
fv3gfs-physics/.* | ||
)$ | ||
- id: flake8 | ||
name: flake8 __init__.py files | ||
files: "__init__.py" | ||
# ignore unused import error in __init__.py files | ||
args: ["--ignore=F401,E203", --config, setup.cfg] | ||
exclude: | | ||
(?x)^( | ||
fv3gfs-physics/.* | ||
)$ |
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.
What you'll need to do here is delete the fv3gfs-physics entry from the first exclude
but leave the init.py one, and delete the exclude
section altogether from the second entry. There should still be two flake8 entries.
# - id: mypy | ||
# args: [ | ||
# --no-strict-optional, | ||
# --ignore-missing-imports, | ||
# --follow-imports, skip # needed so we only test enabled files | ||
# ] | ||
# files: fv3gfs-physics |
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.
You should restore and uncomment this entry.
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.
Thanks for enabling this! I hope it wasn't too much work to update all those constants
references.
|
||
from fv3gfs.physics.global_constants import * | ||
import fv3gfs.physics.global_constants as constants |
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.
On one hand the math looks a little weird with constants
everywhere, but on the other hand it's really helpful to be able to read the code and know what is a constant and what is a variable.
@@ -129,7 +129,7 @@ | |||
mono_prof = False # Perform terminal fall with mono ppm scheme | |||
mp_time = 225.0 # Maximum microphysics timestep (sec) | |||
prog_ccn = 0 # Do prognostic ccn (yi ming's method) | |||
qi0_crt = 8e-05 # Cloud ice to snow autoconversion threshold (highly dependent on horizontal resolution) | |||
qi0_crt = 8e-05 # Cloud ice to snow autoconversion threshold |
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.
is it OK to remove that from this comment? You could continue the comment on the next line.
@@ -1,6 +1,7 @@ | |||
from gt4py.gtscript import BACKWARD, PARALLEL, computation, interval | |||
|
|||
from fv3gfs.physics.global_config import * | |||
from fv3gfs.physics.global_config import FIELD_FLT |
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.
Probably not about this PR but what is a FIELD_FLT and how is it different from a FloatField?
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.
There's no difference actually. Let me clean this up.
|
||
# - pihom: homogeneous freezing of cloud water into cloud ice | ||
# - This is the 1st occurance of liquid water freezing in the split mp process | ||
# - This is the 1st occurance of liquid water freezing |
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.
# - This is the 1st occurance of liquid water freezing | |
# - This is the 1st occurence of liquid water freezing |
description="fv3gfs-physics is a gt4py-based physical parameterization \ | ||
for atmospheric models", |
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.
Nit: You're not wrong but this is the style I see us use most often.
description="fv3gfs-physics is a gt4py-based physical parameterization \ | |
for atmospheric models", | |
description=( | |
"fv3gfs-physics is a gt4py-based physical parameterization " | |
"for atmospheric models" | |
) |
In this PR, linting in
fv3gfs-physics
is enabled.