ImperialCollegeLondon / SWMManywhere

SWMManywhere is used to derive and simulate a sewer network anywhere in the world
https://imperialcollegelondon.github.io/SWMManywhere/
BSD 3-Clause "New" or "Revised" License
2 stars 0 forks source link

Move packages within src and fix extra things #295

Closed dalonsoa closed 5 days ago

dalonsoa commented 5 days ago

Description

Moves swmmanywhere and netcomp to a src folder. And then updates pyproject.toml and a variety of other paths here and there to work with this new layout.

Tests pass, documentation builds and the generated wheel contains the right things (both swmmanywhere and netcomp can be imported after installing the wheel on a fresh environment), but I have not check trying to run a full script so I guess there might be some hidden things that need fixing.

Fixes #290 again

Type of change

Key checklist

Further checks

barneydobson commented 5 days ago

@dalonsoa I don't understand these ruff errors - if I correct them to what is in the failed workflow action - it moves them back when I run pre-commit on my machine...

cheginit commented 5 days ago

For coverage you need:

[tool.coverage.report]
exclude_lines = [
  "if TYPE_CHECKING:",
]
omit = [
  "**/__init__.py",
]
ignore_errors = true

[tool.coverage.paths]
source = [ "src", "*/site-packages" ]
omit = [
  "**/__init__.py",
]

[tool.coverage.run]
branch = true
source = [
  "swmmanywhere",
]
omit = [
  "**/__init__.py",
]

Also, instead of:

only-include = ["src"]
sources = ["src"]

you can just do:

packages = [ "src" ]
dalonsoa commented 5 days ago

@dalonsoa I don't understand these ruff errors - if I correct them to what is in the failed workflow action - it moves them back when I run pre-commit on my machine...

Not sure. I've just re-run pre-commit locally and all works now.

barneydobson commented 5 days ago

I followed advice in : https://github.com/astral-sh/ruff/issues/5667 - not sure if that's what did it but the pre-commit seemed to correct itself now..

not sure this is satisfying as pre-commit will now fail locally...

dalonsoa commented 5 days ago

For coverage you need:

[tool.coverage.report]
exclude_lines = [
  "if TYPE_CHECKING:",
]
omit = [
  "**/__init__.py",
]
ignore_errors = true

[tool.coverage.paths]
source = [ "src", "*/site-packages" ]
omit = [
  "**/__init__.py",
]

[tool.coverage.run]
branch = true
source = [
  "swmmanywhere",
]
omit = [
  "**/__init__.py",
]

Could you elaborate why we need all of these?

dalonsoa commented 5 days ago

I followed advice in : astral-sh/ruff#5667 - not sure if that's what did it but the pre-commit seemed to correct itself now

I enabled precommit.ci, which heals some errors automatically, creating new commits with the fixes. See #296

barneydobson commented 5 days ago

I followed advice in : astral-sh/ruff#5667 - not sure if that's what did it but the pre-commit seemed to correct itself now

I enabled precommit.ci, which heals some errors automatically, creating new commits with the fixes. See #296

Not sure that fixes this though - since it is failing locally but passing after pre commit heal?

dalonsoa commented 5 days ago

Runs locally for me locally on the latest version of this branch with no problems.

dalonsoa commented 5 days ago

Everything nice and green :)

image

cheginit commented 5 days ago

Could you elaborate why we need all of these?

Coverage doesn't work nice with src-layout by default, so some of these options are required to make it correctly detect the lines and report. Also, I usually omit the init to clean up the report.

dalonsoa commented 5 days ago

Done!

barneydobson commented 5 days ago

I dunno - pre-commit run --all-files is still doing ruff corrections on this branch on my machine - I will try on a different machine, maybe I have some weirdness going on on here

cheginit commented 5 days ago

OK, there's an important missing block:

license-files = { paths = ["LICENSE", "src/netcomp/LICENSE.txt"] }