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

Add linting for python code #34

Merged
merged 4 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,43 @@ repos:
# args: [--checks=readability-magic-numbers,--warnings-as-errors=*]
# - id: cppcheck
# - id: include-what-you-use

- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.3.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer

# Can run individually with `pre-commit run isort --all-files`
- repo: https://github.com/PyCQA/isort
rev: 5.12.0
hooks:
- id: isort
args: ["--settings-file=components/omega/python_lint.cfg"]

# Can run individually with `flynt [file]` or `flynt [source]`
- repo: https://github.com/ikamensh/flynt
rev: '0.78'
hooks:
- id: flynt
args: ["--fail-on-change", "--verbose", "--line-length=79"]
require_serial: true

# Can run individually with `pre-commit run flake8 --all-files`
# Need to use flake8 GitHub mirror due to CentOS git issue with GitLab
# https://github.com/pre-commit/pre-commit/issues/1206
- repo: https://github.com/pycqa/flake8
rev: 6.0.0
hooks:
- id: flake8
args: ["--config=components/omega/python_lint.cfg"]
additional_dependencies: [flake8-isort]

# Can run individually with `pre-commit run mypy --all-files`
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.991
hooks:
- id: mypy
args: ["--config=components/omega/python_lint.cfg", "--show-error-codes"]
verbose: true
additional_dependencies: ['types-requests']
2 changes: 1 addition & 1 deletion components/omega/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
include(${CMAKE_CURRENT_SOURCE_DIR}/OmegaBuild.cmake)


# handle build modes
# handle build modes
if (NOT DEFINED PROJECT_NAME)
# enter standalone build

Expand Down
10 changes: 5 additions & 5 deletions components/omega/OmegaBuild.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ set(OMEGA_DEFAULT_BUILD_TYPE Release) # Debug or Release
# Public variables #
###########################
macro(setup_common_variables)

option(OMEGA_DEBUG "Turn on error message throwing (default OFF)." OFF)

if(NOT DEFINED OMEGA_CXX_FLAGS )
Expand All @@ -32,7 +32,7 @@ endmacro()
# Preset Standalone build #
###########################
macro(preset)

# set CMAKE_CXX_COMPILER from OMEGA_CXX_COMPILER
if(OMEGA_CXX_COMPILER)
execute_process(COMMAND which ${OMEGA_CXX_COMPILER}
Expand Down Expand Up @@ -68,7 +68,7 @@ macro(setup_standalone_build)
endif()

set(OMEGA_BUILD_MODE "STANDALONE")
set(OMEGA_BUILD_EXECUTABLE ON)
set(OMEGA_BUILD_EXECUTABLE ON)

endmacro()

Expand Down Expand Up @@ -114,7 +114,7 @@ macro(update_variables)

else()
set(YAKL_ARCH ${OMEGA_ARCH})

if(OMEGA_${YAKL_ARCH}_FLAGS)
set(YAKL_${YAKL_ARCH}_FLAGS ${OMEGA_${YAKL_ARCH}_FLAGS})
endif()
Expand Down Expand Up @@ -171,7 +171,7 @@ macro(wrap_outputs)
if(OMEGA_INSTALL_PREFIX)

install(TARGETS ${OMEGA_LIB_NAME} LIBRARY DESTINATION "${OMEGA_INSTALL_PREFIX}/lib")

if(OMEGA_BUILD_EXECUTABLE)
install(TARGETS ${OMEGA_EXE_NAME} RUNTIME DESTINATION "${OMEGA_INSTALL_PREFIX}/bin")
endif()
Expand Down
3 changes: 1 addition & 2 deletions components/omega/README.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
# The Ocean Model for E3SM Global Applications (OMEGA)

OMEGA is the ocean component planned for v4 of the
OMEGA is the ocean component planned for v4 of the
Energy Exascale Earth System Model (E3SM). It is
based on the earlier Model for Prediction Across
Scales (MPAS) - Ocean model, but has been completely
redesigned and rewritten for better computational
performance and portability. It will further evolve
as continued physical and numerical improvements are
added.

8 changes: 8 additions & 0 deletions components/omega/dev-conda.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,21 @@

# linting
pre-commit

# c++ linting
clang-format
clang-tools
cppcheck
cpplint
lizard
include-what-you-use

# python linting
isort
flynt
flake8
mypy

# documentation
sphinx
sphinx_rtd_theme
Expand Down
1 change: 0 additions & 1 deletion components/omega/doc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,3 @@ design (design), a User's Guide (userGuide) and a reference manual or
developer's guide (devGuide). These sources are generally built for
display through ReadTheDocs or other mechanisms but are often in
restructuredText format so can be readable individually.

2 changes: 1 addition & 1 deletion components/omega/doc/_templates/versions.html
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@
{%- endif %}
</div>
</div>
{%- endif %}
{%- endif %}
4 changes: 1 addition & 3 deletions components/omega/doc/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@

import os
from datetime import date
import sphinx_rtd_theme

from sphinx.application import Sphinx
from sphinx.transforms.post_transforms import SphinxPostTransform
import sphinx_rtd_theme

# -- Project information -----------------------------------------------------

Expand Down
16 changes: 8 additions & 8 deletions components/omega/doc/design/Broadcast.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ configuration from the master rank to all others.
### 2.1 Requirement: Broadcast all supported data types

We must be able to broadcast variables of I4, I8, R4, R8, Real,
boolean, std::string.
boolean, std::string.

### 2.2 Requirement: Broadcast scalars and vectors

Generally we will be broadcasting scalar values, but will require
vectors of values in some cases.
vectors of values in some cases.

### 2.3 Requirement: Broadcast from any rank

Expand Down Expand Up @@ -63,7 +63,7 @@ interfaces cleaner.

#### 4.2.1 Broadcast from master task in default environment

The first form is the simplest for a broadcast within the default
The first form is the simplest for a broadcast within the default
environment and from the master task:

```c++
Expand All @@ -90,7 +90,7 @@ for the source rank to broadcast from.
```c++
int Broadcast([data type] value, ///< [in] value to be broadcast
const int srcRank ///< [in] rank to broadcast from
); //
); //
```

#### 4.2.3 Broadcast from master rank within a different environment
Expand All @@ -116,14 +116,14 @@ int Broadcast([data type] value, ///< [in] value to be broadcast

#### 4.2.5 Broadcast of vector variables

There will be interfaces identical to the above with the value argument
There will be interfaces identical to the above with the value argument
replaced by `std::vector<type> value` to broadcast a vector of values.

#### 4.2.6 Non-blocking broadcasts

Non-blocking forms of the above for all options will exist that
return a request id. Following the MPI standard, this will all
be named IBroadcast. In addition, an IBroadcastWait will be
be named IBroadcast. In addition, an IBroadcastWait will be
included to wait for the request to complete. A non-blocking
sequence would look like:

Expand All @@ -140,13 +140,13 @@ int err = IBroadcastWait(myReqID);
A multi-processor test driver will initialize variables of all supported
types to zero or empty values. The master rank will then set a non-zero
value and broadcast to the remaining ranks. Each rank will verify the
new value is the expected one.
new value is the expected one.
- tests requirement 2.1

### 5.2 Test vectors of all types

Repeat the above with vectors of all types using a set of non-zero
values.
values.
- tests requirement 2.2

### 5.3 Test broadcast from other ranks
Expand Down
52 changes: 26 additions & 26 deletions components/omega/doc/design/Config.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ include YAML, JSON, XML.
### 2.3 Requirement: Archiving and provenance

The entire model configuration must be able to be saved and archived to
act as model provenance.
act as model provenance.

### 2.4 Requirement: Internal accessibility

Expand Down Expand Up @@ -61,7 +61,7 @@ or includes in the main configuration file to make the dependency clear.

### 2.9 Required: Hierarchy or grouping

For both readability and for easier encapsulation by modules/classes, the
For both readability and for easier encapsulation by modules/classes, the
configuration should be organized in a logical way to make it clear where
specific configuration variables are to be used. Parameters primarily or
specifically associated with a module or class should be in a grouping
Expand All @@ -71,7 +71,7 @@ of those parameters with the configuration.

Because the configuration will also be used for provenance, it is likely
that the configuration input file will need to be read by other languages
outside of the OMEGA C++ model. This requires an ability to parse a
outside of the OMEGA C++ model. This requires an ability to parse a
configuration input file from other common languages (eg python).

### 2.11 Requirement: Optional or missing values
Expand All @@ -89,15 +89,15 @@ If extra or unexpected values are encountered, they will be ignored.
### 2.13 Desired: Automated generation of default input and error checking

While the source code defines the configuration variables, it would
be desirable to have a means to extract from the source code what the
be desirable to have a means to extract from the source code what the
code is expecting into a default input config file. This would also
enable some external error checking for missing or extra entries.

### 2.14 Desired: Acceptable values

For users modifying an input configuration, it would be desirable
to document the acceptable values or range of values that each
variable can be assigned.
variable can be assigned.

## 3 Algorithmic Formulation

Expand All @@ -120,9 +120,9 @@ Because extracting variables from a large and complex config
structure is less efficient, we plan for a single reading of the
full configuration. Each initialization routine in OMEGA will then
extract needed variables and manipulate them as needed for the later
forward integration. In this implementation, we will read/write the
forward integration. In this implementation, we will read/write the
configuration from a master rank and each initialization routine
within OMEGA will manipulate and broadcast necessary variables
within OMEGA will manipulate and broadcast necessary variables
across ranks for parallel execution. Because not all variables will
need to be broadcast and many others will be converted to more
efficient types (eg string options to logical or enums), we believe
Expand All @@ -131,7 +131,7 @@ structure and manipulating afterward.

### 4.1 Data types and parameters

#### 4.1.1 Parameters
#### 4.1.1 Parameters

There are no global parameters or shared constants.

Expand All @@ -143,15 +143,15 @@ We define a Config type, which is actually an alias of YAML::node:
using Config = YAML::node;
```

from the yaml-cpp library. A YAML node is more fully and accurately
defined in the YAML specification, but for the purposes of this
document, our configuration is represented in YAML as a set of nested
map nodes, where a map is simply a keyword-value pair. At the lowest
level, these nodes are the simple `variable-name: value` maps. The next
level up is a map of the module name to the collection of maps associated
with the module. The root node corresponds to the full model configuration
from the yaml-cpp library. A YAML node is more fully and accurately
defined in the YAML specification, but for the purposes of this
document, our configuration is represented in YAML as a set of nested
map nodes, where a map is simply a keyword-value pair. At the lowest
level, these nodes are the simple `variable-name: value` maps. The next
level up is a map of the module name to the collection of maps associated
with the module. The root node corresponds to the full model configuration
and is simply a collection of all those module maps. I/O stream/file
configuration will be part of this configuration in a design TBD later
configuration will be part of this configuration in a design TBD later
but will be a similar hierarchy under the full omega config node.

An example YAML input file might then look like:
Expand Down Expand Up @@ -211,7 +211,7 @@ to be associated with Config.
#### 4.2.1 File read and master config

The most common use case should be creating a Config by reading a
YAML configuration file using:
YAML configuration file using:

```c++
Config omegaConfig = ConfigRead("omega.yml");
Expand Down Expand Up @@ -241,7 +241,7 @@ useRefWidth = ConfigGet(hmixConfig, "hmixUseRefWidth", iErr);
where there is a retrieval function for all supported Omega data types:
bool, I4, I8, R4, R8, Real, std::string. These retrievals are just
overloaded wrappers around the YAML form: `configName["varName"].as<type>`
with some error checking and reporting. Rather than a templated form,
with some error checking and reporting. Rather than a templated form,
we use simple overloading to keep a cleaner interface. If the variable
or config is missing, these functions will print an error message and
return with a non-zero error argument.
Expand All @@ -263,7 +263,7 @@ being used because the entry is missing.

While the intent is for all config variables to be set using the config
file read interface above, the capability modify a value is also
required. The syntax is essentially the inverse of the get/retrieval
required. The syntax is essentially the inverse of the get/retrieval
above. Similar to that case, the sub-group will need to be retrieved first.

```c++
Expand All @@ -272,7 +272,7 @@ ConfigSet(hmixConfig, "hmixRefWidth", 10.0e3, iErr);
ConfigSet(hmixConfig, "hmixUseRefWidth", true, iErr);
```

There will be overloaded interfaces for each supported type. For
There will be overloaded interfaces for each supported type. For
literals (as in the example above), they will be cast to an appropriate
type according to C++ default type conversion and will be converted
to the desired type on retrieval (YAML internal storage is ignorant of
Expand All @@ -297,7 +297,7 @@ ConfigAdd(hmixConfig, "hmixUseRefWidth", true, iErr);
ConfigAdd(omegaConfig, hmixConfig); // add new subgroup to parent
```

There will be overloaded interfaces for each supported type. For
There will be overloaded interfaces for each supported type. For
literals (as in the example above), they will be cast to an appropriate
type according to C++ default type conversion and will be converted
to the desired type on retrieval (YAML internal storage is ignorant of
Expand All @@ -306,7 +306,7 @@ the type and only performs the type cast on retrieval).
#### 4.2.4 Existence

It is not expected that a user would test the existence since the
Get/Set functions will perform the test internally. However, to
Get/Set functions will perform the test internally. However, to
satisfy requirement 2.11, we will add a function to test the
existence of an entry, given a config or sub-config.
Using the hmix example again:
Expand Down Expand Up @@ -373,8 +373,8 @@ This block would follow Doxygen-like format and look something like:
```

The block between the ConfigInput lines could be extracted verbatim
and written (with proper indenting) into a fully documented yaml
default input file or could be extracted and the `#`-delimited
and written (with proper indenting) into a fully documented yaml
default input file or could be extracted and the `#`-delimited
comments stripped for a more concise yaml input file.
In addition, we may be able to similarly extract the same info for
the User and Developer Guides.
Expand All @@ -384,7 +384,7 @@ the User and Developer Guides.
The selection of YAML automatically satisfies Requirements 2.1, 2.2 and
2.10. Requirement 2.8 will be enforced by Omega development. The other
requirements will be tested with a parallel unit test driver that performs
the following tests in order and will output the test name and a
the following tests in order and will output the test name and a
PASS/FAIL for use with CTest or other testing frameworks.


Expand All @@ -396,7 +396,7 @@ Create an empty config on master rank using default constructor.
### 5.2 Test Set function

Add configuration variables to the empty config with at least one of
each supported type with a few extras to test behavior for other
each supported type with a few extras to test behavior for other
requirements. Also add multiple levels of hierarchy and a large
enough number of subgroups and parameters to simulate a full omega
config.
Expand Down
Loading