openmethane / setup-wrf

Setup the WRF model
0 stars 0 forks source link

Attrs class for CMAQ config #19

Closed crdanielbusch closed 3 weeks ago

crdanielbusch commented 3 weeks ago
crdanielbusch commented 3 weeks ago

@lewisjared I think this is ready to be reviewed.

One thing I haven't dealt with yet is checking the formats for the configuration, so specifically: do some of the values have to be in a list or not, e.g. ["d01"] or "d01". I can say that it works if they are all in a list and does not work if they are all not in a list. I haven't found a quick way to check the individual values as everything has to run in the Docker container and I'm not sure if it's worth the effort.

As discussed, the tests run in the ci as separate step now. I will look into running them with the integration test in parallel tomorrow.

crdanielbusch commented 3 weeks ago

Nice. Can you also run ruff before merging

I usually go through all the files and click on Code -> Reformat Code (with the ruff plugin). Is there a better way to do this?

lewisjared commented 3 weeks ago

Usually a pre-commit hook or make ruff-fixes

if it isn't already in the Makefile add:

.PHONY: ruff-fixes
ruff-fixes:  # Run ruff on the project
    # Run the formatting first to ensure that is applied even if the checks fail
    poetry run ruff format .
    poetry run ruff check --fix .
    poetry run ruff format .