Skip to content

Commit

Permalink
JP-3769: Style rules example case (#9081)
Browse files Browse the repository at this point in the history
  • Loading branch information
@emolter
emolter authored Jan 29, 2025
1 parent 59f9662 commit d95188f
Show file tree
Hide file tree
Showing 32 changed files with 2,597 additions and 2,261 deletions.
88 changes: 81 additions & 7 deletions .pre-commit-config.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ exclude: ".*\\.asdf$"

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0
rev: v5.0.0
hooks:
- id: check-added-large-files
- id: check-ast
Expand All @@ -17,13 +17,87 @@ repos:
# - id: end-of-file-fixer
# - id: trailing-whitespace
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: 'v0.5.2'
rev: 'v0.9.2'
hooks:
- id: ruff
args: ["--fix"]
# - id: ruff-format
- repo: https://github.com/PyCQA/bandit
rev: 1.7.9
- id: ruff-format
- repo: https://github.com/numpy/numpydoc
rev: v1.8.0
hooks:
- id: bandit
args: ["-r", "-ll", "-x", "jwst/*test*,jwst/**/*test*,jwst/fits_generator", "jwst"]
- id: numpydoc-validation
exclude: |
(?x)^(
jwst/assign_mtwcs/.* |
jwst/assign_wcs/.* |
jwst/associations/.* |
jwst/background/.* |
jwst/badpix_selfcal/.* |
jwst/barshadow/.* |
jwst/charge_migration/.* |
jwst/clean_flicker_noise/.* |
jwst/combine_1d/.* |
jwst/coron/.* |
jwst/cube_build/.* |
jwst/cube_skymatch/.* |
jwst/dark_current/.* |
jwst/datamodels/.* |
jwst/dq_init/.* |
jwst/emicorr/.* |
jwst/engdblog/.* |
jwst/exp_to_source/.* |
jwst/extract_1d/.* |
jwst/extract_2d/.* |
jwst/firstframe/.* |
jwst/fits_generator/.* |
jwst/flatfield/.* |
jwst/fringe/.* |
jwst/gain_scale/.* |
jwst/group_scale/.* |
jwst/guider_cds/.* |
jwst/imprint/.* |
jwst/ipc/.* |
jwst/jump/.* |
jwst/lastframe/.* |
jwst/lib/.* |
jwst/linearity/.* |
jwst/master_background/.* |
jwst/model_blender/.* |
jwst/mrs_imatch/.* |
jwst/msaflagopen/.* |
jwst/nsclean/.* |
jwst/outlier_detection/.* |
jwst/pathloss/.* |
jwst/persistence/.* |
jwst/photom/.* |
jwst/pipeline/.* |
jwst/pixel_replace/.* |
jwst/ramp_fitting/.* |
jwst/refpix/.* |
jwst/regtest/.* |
jwst/resample/.* |
jwst/reset/.* |
jwst/residual_fringe/.* |
jwst/rscd/.* |
jwst/saturation/.* |
jwst/scripts/.* |
jwst/skymatch/.* |
jwst/source_catalog/.* |
jwst/spectral_leak/.* |
jwst/srctype/.* |
jwst/stpipe/.* |
jwst/straylight/.* |
jwst/superbias/.* |
jwst/tests/.* |
jwst/tso_photometry/.* |
jwst/tweakreg/.* |
jwst/wavecorr/.* |
jwst/wfs_combine/.* |
jwst/wfss_contam/.* |
jwst/white_light/.* |
jwst/conftest.py |
.*/tests/.* |
setup.py |
pytest_jwst/.* |
docs/.*
)$
220 changes: 220 additions & 0 deletions .ruff.toml
View file

Large diffs are not rendered by default.

205 changes: 152 additions & 53 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,58 +96,85 @@ there are other options.

1. Create a conda environment.

It is good practice to maintain different environments for different versions
of JWST and its dependencies. You will likely want to maintain one, for example,
for the latest released version of JWST (i.e. what you get by doing `pip install jwst`),
as well as one for development. Assuming the user has conda [installed](https://docs.conda.io/projects/conda/en/latest/user-guide/install/index.html),
here we will create a new conda environment called 'jwst_dev' where we can install the new branch of our cloned repository.

>> conda create -n jwst_dev python


Doing this will create a new environment with just some basic packages
(i.e setuptools, pip) installed.

2. Installing `jwst` in this environment

Make sure you are in your new environment:

>> conda activate jwst_dev

And now in the top level of your local `jwst` repository, ensuring you're
on the 'my_feature' branch:

>> pip install -e .
This will install `jwst` from this cloned source code in 'editable' mode,
meaning that you can import the code from this directory when within a Python
session. This makes it easier for development because you can have the code
you're editing somewhere convenient in your file system vs. with other packages
in 'site-packages'. If you cloned the repository on your Desktop, for example,
you can modify it there and Python will know that is where the source code is
when you're importing it within a Python session.

*Note* : If you use it, make sure to install iPython in your new environment
as well. Otherwise, it will pick up packages from the base environment instead.
It is good practice to maintain different environments for different versions
of JWST and its dependencies. You will likely want to maintain one, for example,
for the latest released version of JWST (i.e. what you get by doing `pip install jwst`),
as well as one for development. Assuming the user has conda [installed](https://docs.conda.io/projects/conda/en/latest/user-guide/install/index.html),
here we will create a new conda environment called 'jwst_dev' where we can install the new branch of our cloned repository.

>> conda create -n jwst_dev python


Doing this will create a new environment with just some basic packages
(i.e setuptools, pip) installed.

2. Install `jwst` in this environment

Make sure you are in your new environment:

>> conda activate jwst_dev

And now in the top level of your local `jwst` repository, ensuring you're
on the 'my_feature' branch:

>> pip install -e ".[contrib]"

This will install `jwst` from this cloned source code in 'editable' mode,
meaning that you can import the code from this directory when within a Python
session. This makes it easier for development because you can have the code
you're editing somewhere convenient in your file system vs. with other packages
in 'site-packages'. If you cloned the repository on your Desktop, for example,
you can modify it there and Python will know that is where the source code is
when you're importing it within a Python session. This command will also install
the optional testing and documentation dependencies, including pre-commit.

---
**Note:** If you use it, make sure to install iPython in your new environment
as well. Otherwise, it will pick up packages from the base environment instead.

---

3. Set up pre-commit checks

All of the coding style rules [described below](#code-style) can be checked
automatically when you make a git commit using our provided pre-commit hook for git;
for more information see: [Git Hooks](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks#_git_hooks),
[pre-commit](https://pre-commit.com/). We strongly encourage setting up
and using these hooks to ensure that contributed code always meets our coding style standards.
Pre-commit should already be installed in your conda environment after running
`pip install -e ".[contrib]"`, but if not, simply `pip install pre-commit`.
Navigate to the root directory of the jwst repository, then run

>> pre-commit install

This will set up the pre-commit hook in
`jwst/.git/hooks/`, and make it so that any modified files will be checked by the pre-commit
workflow every time `git commit` is run.

---
**NOTE:** The changes made by `pre-commit` will not be automatically staged,
so you will need to review and re-stage any files that pre-commit has changed.

---

### Step 4: Making code changes

Now that you've forked, cloned, made a new branch for your feature, and installed
it in a new environment for development of `jwst`, you are ready to make changes
to the code. As you make changes, make sure to `git commit -m <"some message">` frequently
(in case you need to undo something by reverting back to a previous commit - you
cant do this if you commit everything at once!). After you've made your desired
can't do this if you commit everything at once!). Changes should be tested locally
using pytest; see [Writing and Running Unit Tests](#writing-and-running-unit-tests)
for details.

After you've made your desired
changes, and committed these changes, you will need to push them online to your
'remote' fork of `jwst`:

>> git push origin my_feature

If the changes are significant, please make an entry in `CHANGES.rst` in the top
level `jwst` directory with a short description of the changes you've made and, once
you open a pull request, add the corresponding PR number.



### Step 4: Opening a pull request
### Step 5: Opening a pull request

Now, you can open a pull request on the main branch of the upstream `jwst` repository.

Expand All @@ -158,21 +185,23 @@ with your recently pushed changes. You can also open a pull request from the
Select your fork and your 'my_feature' branch, and open a pull request against
the 'main' branch.

2. There is now a checklist of items that need to be done before your PR can be merged.
* The continuous integration (CI) tests must complete and pass. The CI
runs several different checks including running the unit tests, ensuring
the documentation builds, checking for code style issues (see the [PEP8](https://peps.python.org/pep-0008/) style guide),
and ensuring any changes are covered by unit tests. The CI runs upon opening
a PR, and will re-run any time you push commits to that branch.
* Our code style checker uses [ruff](https://docs.astral.sh/ruff/), and using
`ruff check` locally can be helpful to identify style issues before opening a PR.
* You will need to add a change log entry in changes/PRID.fragmenttype.rst
2. Ensure the GitHub CI checks are all passing.
* Unit tests will be run, and unit test coverage will be checked for
any new code. See [Writing and Running Unit Tests](#writing-and-running-unit-tests)
to learn how to run and debug the unit tests if you're seeing failures.
* Various code style checks will be run (see [Code Style](#code-style)). The
pre-commit hook described above is helpful for catching and debugging these locally.
* You will need to add a change log entry in `changes/PRID.fragmenttype.rst`
if your contribution is a new feature or bug fix.
An entry is not required for small fixes like typos.
* Your PR will need to be reviewed and approved by at least two maintainers.
They may require changes from you before your code can be merged, in which
case you will need to go back and make these changes and push them (they will
automatically appear in the PR when they're pushed to origin/my_feature).

3. Ensure all the items in the **Tasks** checklist on the PR are completed.
Instructions for how to do this are included in the checklist itself.

4. Your PR will need to be reviewed and approved by at least two maintainers.
They may require changes from you before your code can be merged, in which
case you will need to go back and make these changes and push them (they will
automatically appear in the PR when they're pushed to origin/my_feature).


# Advanced Contribution Instructions
Expand Down Expand Up @@ -229,6 +258,7 @@ directory of `jwst` on your my_feature branch:

>> pip install -e ".[docs]"

(Note the doc dependencies are also included when installing `jwst` with the `[contrib]` tag).
Now, with the correct documentation dependencies installed, you can attempt to build
the documentation locally. To do this, enter into the `jwst/docs` subdirectory and do:

Expand Down Expand Up @@ -270,7 +300,9 @@ for running tests.
>> pip install -e ".[test]"

This will install the optional 'test' dependencies specified in `pyproject.toml` that
don't install by default. The package `pytest` is one of these and is what's used
don't install by default. (Note these test dependencies are also included when
installing `jwst` with the `[contrib]` tag).
The package `pytest` is one of these and is what's used
to run the tests. `pytest` searches through all the directories in your repository
(underneath the directory from which it was invoked command line) and looks for any
directories called 'test' or .py files with the word 'test' in the name. Functions
Expand Down Expand Up @@ -341,5 +373,72 @@ version in `pyproject.toml` to:
And similarly, in `stcal`, change the required `jwst` version to:

>> jwst @ git+https://github.com/<your_username>/jwst.git@<your_branch>

Let the CI run and ensure it passes, comment this in your PR and make sure the reviewers
confirm, and then change the versions back before your PR is merged (which will again cause the CI to fail, but that’s OK).

## Code style

We use a pre-commit CI workflow to ensure that the code and docstring style of the `jwst` repository
remains uniform and conforms to certain standards. We recommend checking these using `pre-commit`
as described in the [Installing JWST for Development](Step-3-Installing-jwst-for-development)
section. For additional information about any of these individual style checkers,
see their documentation linked below.
Our pre-commit Git hook, also described in the
[Installing JWST for Development](Step-3-Installing-jwst-for-development) section,
is designed to help contributors run all the checks on their contributions every time they commit.

The following three style checks are performed:

* **PEP8-compliant code**

The code style for the `jwst` repository generally conforms to
[PEP8](https://peps.python.org/pep-0008/), and the code style rules are enforced
using [Ruff](https://docs.astral.sh/ruff/). To run these checks standalone,
use the command

>> pre-commit run ruff

from within the `jwst` repository. Ruff will automatically pick up the appropriate configuration from the `.ruff.toml` and `pre-commit-config.yaml` files,
and perform only the checks that are turned on for our repository. To run ruff's
auto-formatter, which automatically fixes simple things like single vs double quotes, whitespace, etc., use the command

>> pre-commit run ruff-format

---
**Note:** If you run `ruff format .` from the top-level `jwst/` folder, it will re-format the entire code base.
Please do not do this; it makes pull requests more challenging to review. It's recommended to apply `ruff-format` through `pre-commit`.

---

* **Numpy docstring style**

The docstring style for the `jwst` repository generally conforms to the
[Numpy style guide](https://numpydoc.readthedocs.io/en/latest/format.html), and the docstring
style rules are enforced using [numpydoc-validation](https://numpydoc.readthedocs.io/en/latest/validation.html).
To run these checks locally, use the command

>> pre-commit run numpydoc-validation


* **PEP-compliant type hints**

The majority of the `jwst` repository does *not* have any type hints, and type hints are *not*
required for contributions. If type hints are used, though, their compliance with
[PEP-484](https://peps.python.org/pep-0484/) standards
is enforced using [mypy](https://mypy.readthedocs.io/en/stable/index.html).
To run these checks locally, first `pip install mypy` then use the command

>> mypy .

from within the `jwst` repository. This check is not run through pre-commit;
it's instead handled separately from a different CI workflow. This means that if you are
using type hints, you may encounter a situation where `pre-commit` passes locally but
the CI checks fail on a pull request.

---
**Note:** At time of writing, many submodules in the repository do not yet conform to the style rules;
however, we have made it a priority to get the whole code base up to standard in the next few months,
and any new contributions must now follow the style rules as indicated.

---
2 changes: 2 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
[![Build Status](https://github.com/spacetelescope/jwst/workflows/CI/badge.svg)](https://github.com/spacetelescope/jwst/actions)
[![codecov](https://codecov.io/gh/spacetelescope/jwst/branch/main/graph/badge.svg?token=Utf5Zs9g7z)](https://codecov.io/gh/spacetelescope/jwst)
[![Documentation Status](https://readthedocs.org/projects/jwst-pipeline/badge/?version=latest)](http://jwst-pipeline.readthedocs.io/en/latest/?badge=latest)
[![Pre-Commit](https://img.shields.io/badge/pre--commit-enabled-brightgreen?logo=pre-commit&logoColor=white)](https://github.com/pre-commit/pre-commit)
[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff)
[![Powered by STScI Badge](https://img.shields.io/badge/powered%20by-STScI-blue.svg?colorA=707170&colorB=3e8ddd&style=flat)](http://www.stsci.edu)
[![Powered by Astropy Badge](http://img.shields.io/badge/powered%20by-AstroPy-orange.svg?style=flat)](http://www.astropy.org/)
[![DOI](https://zenodo.org/badge/60551519.svg)](https://zenodo.org/badge/latestdoi/60551519)
Expand Down
1 change: 1 addition & 0 deletions changes/9081.general.rst
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add code style rules, using AMI module as an example case
2 changes: 1 addition & 1 deletion docs/jwst/ami_analyze/ami_test.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ ________________________

hexee module tests:
+++++++++++++++++++
- Test of g_eeAG()
- Test of g_eeag()
Calculate the Fourier transform of one half of a hexagon that is
bisected from one corner to its diametrically opposite corner.
- Test of glimit()
Expand Down
2 changes: 2 additions & 0 deletions jwst/ami/__init__.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"""Handle aperture mask imaging (AMI) data."""

from .ami_analyze_step import AmiAnalyzeStep

# from .ami_average_step import AmiAverageStep
Expand Down
Loading

0 comments on commit d95188f

Please sign in to comment.