Skip to content

Commit

Permalink
todos to issues (#51)
Browse files Browse the repository at this point in the history
* remove todo
#42

* remove todo
#42

* remove todos
#47 and #42

* remove todo
#42

* remove todos
#49 and #42

* remove todos
#50 and #42

* improve code validation

* adapt code validation code duplication is allowed
  • Loading branch information
Kwasniok authored Sep 30, 2021
1 parent ac44ff1 commit 39bbe78
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 26 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/pylint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,4 @@ jobs:
pip install pylint
- name: Analysing the code with pylint
run: |
# TODO: code duplication should be allowed for unittest
pylint src
pylint --disable=R0801 src
2 changes: 1 addition & 1 deletion src/nerte/geometry/cylindircal_swirl_geometry_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def geodesic_equation(
ray: TangentialVectorDelta,
) -> TangentialVectorDelta:
# pylint: disable=C0103
# TODO: revert when mypy bug was fixed
# MYPY-BUG: revert when mypy bug was fixed
# see https://github.com/python/mypy/issues/2220
# r, _, z = ray.point_delta
# v_r, v_phi, v_z = ray.vector_delta
Expand Down
7 changes: 4 additions & 3 deletions src/nerte/values/linalg.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def dot(
return np.dot(
vec1._v,
np.dot(metric.matrix()._m, vec2._v), # type: ignore[no-untyped-call]
) # TODO: optimize
) # POSSIBLE-OPTIMIZATION: hard code


def cross(
Expand Down Expand Up @@ -270,8 +270,9 @@ def length(vec: AbstractVector, metric: Optional[Metric] = None) -> float:
Returns the length of the vector (with respect to an orthonormal basis).
"""
if metric is None:
# POSSIBLE-OPTIMIZATION: hard code
return np.linalg.norm(vec._v) # type: ignore[no-untyped-call]
return dot(vec, vec, metric) ** 0.5 # TODO: optimize
return dot(vec, vec, metric) ** 0.5


def normalized(
Expand All @@ -285,7 +286,7 @@ def normalized(
return _abstract_vector_from_numpy(vec._v * (dot(vec, vec) ** -0.5))
# NOTE: DON'T use this:
# return Vector.__from_numpy((1 / np.linalg.norm(vec._v)) * vec._v)
return vec / length(vec, metric) # TODO: optimize
return vec / length(vec, metric) # POSSIBLE-OPTIMIZATION: hard code


def are_linear_dependent(vectors: tuple[AbstractVector, ...]) -> bool:
Expand Down
2 changes: 1 addition & 1 deletion src/nerte/values/manifold.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class OutOfDomainError(ValueError):
pass


# TODO: Implement when type families are supported in python.
# FUTURE: Implement when type families are supported in python.

# class ManifoldTypeFamily[N:int]():
# # pylint: disable=W0107
Expand Down
2 changes: 1 addition & 1 deletion src/nerte/values/manifolds/cartesian_swirl.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def _christoffel_1(
sin_2alpha = math.sin(2 * alpha)
cos_3alpha = math.cos(3 * alpha)
sin_3alpha = math.sin(3 * alpha)
# TODO: potential speed-up: symmetric matrices
# POSSIBLE-OPTIMIZATION: symmetric matrices
return (
AbstractMatrix(
AbstractVector(
Expand Down
5 changes: 0 additions & 5 deletions src/nerte/values/manifolds/cylindrical_swirl_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ def test_metric(self) -> None:
)


# TODO: edge case + non-edge case
class CylindircSwirlGeodesicEquationTest(BaseTestCase):
def setUp(self) -> None:
self.swirl = 0.0
Expand Down Expand Up @@ -149,7 +148,6 @@ def cylin_next(x: TangentialVectorDelta) -> TangentialVectorDelta:
)


# TODO: edge case + non-edge case
class CylindricalCoordinatesTransfomrationTest(BaseTestCase):
def setUp(self) -> None:
self.swirl = 0.0
Expand Down Expand Up @@ -198,7 +196,6 @@ def test_cylindric_swirl_to_carthesian_coords(self) -> None:
cylindric_swirl_to_carthesian_coords(self.swirl, coords)


# TODO: edge case + non-edge case
class CylindricSwirlVectorTransfomrationTest(BaseTestCase):
def setUp(self) -> None:
self.swirl = 0.0
Expand Down Expand Up @@ -307,7 +304,6 @@ def test_vector_length_preservation(self) -> None:
self.assertAlmostEqual(cylin_len, carth_len)


# TODO: edge case + non-edge case
class CylindricalTangentialVectorTransfomrationTest(BaseTestCase):
def setUp(self) -> None:
self.swirl = 0.0
Expand Down Expand Up @@ -519,7 +515,6 @@ def test_plane_tangential_space_domain(self) -> None:
self.infinite_plane.tangential_space(coords)


# TODO: edge case + non-edge case
class PlanePropertiesTest(BaseTestCase):
# pylint: disable=R0902
def setUp(self) -> None:
Expand Down
54 changes: 41 additions & 13 deletions validate_for_commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ def __exit__(self, exc_type, exc_val, exc_tb) -> None: # type: ignore

# src must exist
if not os.path.isdir("src"):
print("Code base invalid. No directory 'src' found.")
print("ERROR: Code base invalid. No directory 'src' found.")
sys.exit(1)

# src must be readable
if not os.access("src", os.R_OK):
print("Code base invalid. Cannot read directory 'src'.")
print("ERROR: Code base invalid. Cannot read directory 'src'.")
sys.exit(1)


Expand All @@ -59,27 +59,53 @@ def run_mypy() -> None:
stderr=sys.stderr,
)
if res.returncode != 0:
print("Code base invalid. Mypy failed.")
print("ERROR: Code base invalid. Mypy failed.")
sys.exit(1)


def run_pylint(disable: Optional[list[str]] = None) -> None:
def _pylint_disable_str(disable: list[str]) -> str:
if disable is None:
return ""
dis = ",".join(disable)
return f" --disable={dis}"


def _pylint_ignnore_patterns_str(exclude_unittests: bool) -> str:
if exclude_unittests:
return ' --ignore-patterns=".*_unittest.py"'
return ""


def run_pylint(
disable: Optional[list[str]] = None,
exclude_unittests: bool = False,
) -> None:
"""Runs pylint on src."""

if disable is None:
disable = []
disable_str = _pylint_disable_str(disable)
ignore_patterns_str = _pylint_ignnore_patterns_str(exclude_unittests)

print("running pylint ...")
with ChangeDirecory("."):
if disable is None:
dis = ""
else:
dis = ",".join(disable)
res = subprocess.run(
f"pylint --disable={dis} src",
f"pylint{disable_str}{ignore_patterns_str} src",
shell=True,
check=False,
stdout=sys.stdout,
stderr=sys.stderr,
)
if exclude_unittests:
print("INFO: Unit tests were excluded.")
if len(disable) > 0:
print(
"INFO: The following codes were disabled: "
+ ", ".join(disable)
+ "."
)
if res.returncode != 0:
print("Code base invalid. Pylint failed.")
print("ERROR: Code base invalid. Pylint failed.")
sys.exit(1)


Expand All @@ -95,14 +121,14 @@ def run_tests() -> None:
stderr=sys.stderr,
)
if res.returncode != 0:
print("Code base invalid. Tests failed.")
print("ERROR: Code base invalid. Tests failed.")
sys.exit(1)


def run_all() -> None:
"""Full run of checks."""
run_mypy()
run_pylint()
run_pylint(disable=["R0801"])
run_tests()
print("Code base is valid!")
sys.exit(0) # must be 0
Expand All @@ -114,7 +140,9 @@ def run_light() -> None:
run_mypy()
run_tests()
# do pylint last as it often fails in pre-checks
run_pylint(disable=["R0801", "W0511"])
# fix me: W0511
# code duplication: R0801
run_pylint(disable=["R0801"])
print(
"WARNING: Results were obtained in light mode."
" Therefore the code base may not be valid!"
Expand Down

0 comments on commit 39bbe78

Please sign in to comment.