Skip to content

Commit

Permalink
Add --all-files option to precommit command to avoid skipping the test
Browse files Browse the repository at this point in the history
No tests were actually done by the linter action because `pre-commit
run` look only files in the index by default.
When checkout a whole PR, no files are in the index so no files are
checked.

Previously, the job returned something like

	Trim Trailing Whitespace.............................(no files to check)Skipped
	Fix End of Files.....................................(no files to check)Skipped
	Check Yaml...........................................(no files to check)Skipped
    	Check for added large files..........................(no files to check)Skipped
    	Pretty format JSON...................................(no files to check)Skipped
    	isort (python).......................................(no files to check)Skipped
    	black................................................(no files to check)Skipped
    	nbqa-isort...........................................(no files to check)Skipped
    	nbqa-black...........................................(no files to check)Skipped
    	nbqa-pycln...........................................(no files to check)Skipped
    	nbstripout...........................................(no files to check)Skipped

Now the checks are done (and should read as "Passed")
We make a `pre-commit run --all-files` in this commit to have the job
pass.

We also remove the pretty-format-json hook which was tampering with the
notebooks (oddly only remotely on the github runner, not locally on my
laptop), and was not very useful for the repository anyway.
  • Loading branch information
nhuet authored and g-poveda committed Nov 17, 2023
1 parent 574fe52 commit 1ba371e
Show file tree
Hide file tree
Showing 13 changed files with 94 additions and 86 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ jobs:
path: ~/.cache/pre-commit
key: pre-commit|${{ env.pythonLocation }}|${{ hashFiles('.pre-commit-config.yaml') }}
- name: pre-commit checks
run: pre-commit run --show-diff-on-failure --color=always
run: pre-commit run --show-diff-on-failure --color=always --all-files

build-windows:
needs: [setup]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jobs:
path: ~/.cache/pre-commit
key: pre-commit|${{ env.pythonLocation }}|${{ hashFiles('.pre-commit-config.yaml') }}
- name: pre-commit checks
run: pre-commit run --show-diff-on-failure --color=always
run: pre-commit run --show-diff-on-failure --color=always --all-files

build-windows:
needs: [setup]
Expand Down
2 changes: 0 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ repos:
- id: end-of-file-fixer
- id: check-yaml
- id: check-added-large-files
- id: pretty-format-json
args: ["--autofix", "--no-sort-keys", "--indent", "4"]

- repo: https://github.com/psf/black
rev: 22.3.0 # Replace by any tag/version: https://github.com/psf/black/tags
Expand Down
10 changes: 5 additions & 5 deletions docs/contribute.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ Here are the steps to follow:
```shell
poetry env use 3.8.11
```
- Preinstall gym 0.21.0 with appropriate option to avoid an error during installation
(see this [issue](https://github.com/openai/gym/issues/3176)

- Preinstall gym 0.21.0 with appropriate option to avoid an error during installation
(see this [issue](https://github.com/openai/gym/issues/3176)
and this [solution](https://github.com/python-poetry/poetry/issues/3433#issuecomment-840509576)):
```shell
poetry run python -m pip install "pip==22" # starting with pip 23.1, gym 0.21.0 is not intallable anymore
Expand Down Expand Up @@ -132,8 +132,8 @@ as it can also be installed by conda via the conda-forge channel.
poetry self add poetry-dynamic-versioning
```

- Preinstall gym 0.21.0 with appropriate option to avoid an error during installation
(see this [issue](https://github.com/openai/gym/issues/3176)
- Preinstall gym 0.21.0 with appropriate option to avoid an error during installation
(see this [issue](https://github.com/openai/gym/issues/3176)
and this [solution](https://github.com/python-poetry/poetry/issues/3433#issuecomment-840509576)):
```shell
poetry run python -m pip install "pip==22" # starting with pip 23.1, gym 0.21.0 is not intallable anymore
Expand Down
10 changes: 5 additions & 5 deletions docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ pip install -U scikit-decide

## Troubleshooting

You may encounter an [error when installing `gym==0.21.0`](https://github.com/openai/gym/issues/3176) which happens to be a dependency of `scikit-decide[all]`.
This is because its installation does not respect PEP 517 which is enforced by default by last versions of pip and setuptools.
You may encounter an [error when installing `gym==0.21.0`](https://github.com/openai/gym/issues/3176) which happens to be a dependency of `scikit-decide[all]`.
This is because its installation does not respect PEP 517 which is enforced by default by last versions of pip and setuptools.
The solution is to install it beforehand:
```shell
# preinstall gym==0.21.0 with legacy method (python setup.py) because its requirements list is broken
Expand All @@ -145,7 +145,7 @@ pip install -U scikit-decide[all]
```

::: tip Note
Newer versions of gym or [gymnasium](https://gymnasium.farama.org/), typically greater than 0.26 are not yet possible
because of a conflict between [`ray[rllib]`](https://github.com/ray-project/ray/issues/34396)
Newer versions of gym or [gymnasium](https://gymnasium.farama.org/), typically greater than 0.26 are not yet possible
because of a conflict between [`ray[rllib]`](https://github.com/ray-project/ray/issues/34396)
and [`stable-baselines3`](https://github.com/DLR-RM/stable-baselines3/issues/1452).
:::
:::
10 changes: 5 additions & 5 deletions examples/up_bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@
# LICENSE file in the root directory of this source tree.

import sys
from skdecide.hub.domain.up import UPDomain
from skdecide.hub.solver.up import UPSolver
from skdecide.hub.solver.lazy_astar import LazyAstar

import unified_planning
from unified_planning.shortcuts import (
UserType,
BoolType,
OneshotPlanner,
Fluent,
InstantaneousAction,
Not,
OneshotPlanner,
UserType,
)

from skdecide.hub.domain.up import UPDomain
from skdecide.hub.solver.lazy_astar import LazyAstar
from skdecide.hub.solver.up import UPSolver
from skdecide.utils import rollout

# Example 1: Solving a basic example, the same as
Expand Down
1 change: 0 additions & 1 deletion skdecide/hub/domain/flight_planning/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# Copyright (c) AIRBUS and its affiliates.
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

77 changes: 44 additions & 33 deletions skdecide/hub/domain/flight_planning/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from skdecide.hub.space.gym import EnumSpace, ListSpace, MultiDiscreteSpace
from skdecide.utils import match_solvers


class WeatherDate:
day: int
month: int
Expand Down Expand Up @@ -159,7 +160,6 @@ def previous_day(self):
return WeatherDate(day, month, year, forecast=self.forecast)



class State:
"""
Definition of a aircraft state during the flight plan
Expand Down Expand Up @@ -190,11 +190,12 @@ def __hash__(self):
return hash((self.pos, int(self.mass), self.alt, int(self.time)))

def __eq__(self, other):
return (self.pos == other.pos
and int(self.mass) == int(other.mass)
and self.alt == other.alt
and int(self.time) == int(other.time)
)
return (
self.pos == other.pos
and int(self.mass) == int(other.mass)
and self.alt == other.alt
and int(self.time) == int(other.time)
)

def __ne__(self, other):
return (
Expand Down Expand Up @@ -774,7 +775,7 @@ def set_network(
half_vertical_points = nb_vertical_points // 2

distp = (
graph_width * p0.distanceTo(p1) *0.022
graph_width * p0.distanceTo(p1) * 0.022
) # meters, around 2.2%*graphwidth of the p0 to p1 distance

descent_dist = min(
Expand All @@ -785,20 +786,20 @@ def set_network(
),
p0.distanceTo(p1),
) # meters

climb_dist = 220_000 * (
max(self.ac["cruise"]["height"] - p0.height, 0)
/ self.ac["cruise"]["height"]
) # meters

total_distance = p0.distanceTo(p1)
if total_distance < (climb_dist + descent_dist):
climb_dist = total_distance * max(
(climb_dist / (climb_dist + descent_dist)) - 0.1
,0)
(climb_dist / (climb_dist + descent_dist)) - 0.1, 0
)
descent_dist = total_distance * max(
descent_dist / (climb_dist + descent_dist) - 0.1
,0)
descent_dist / (climb_dist + descent_dist) - 0.1, 0
)
possible_altitudes = [cruise_alt_min for k in range(nb_vertical_points)]

else:
Expand All @@ -816,11 +817,15 @@ def set_network(
if climbing_slope:
climbing_ratio = climbing_slope
else:
climbing_ratio = (possible_altitudes[0] / climb_dist if climb_dist != 0 else 0)
climbing_ratio = (
possible_altitudes[0] / climb_dist if climb_dist != 0 else 0
)
if descending_slope:
descending_ratio = descending_slope
else:
descending_ratio = (possible_altitudes[0] / descent_dist if descent_dist != 0 else 0)
descending_ratio = (
possible_altitudes[0] / descent_dist if descent_dist != 0 else 0
)
# Initialisation of the graph matrix
pt = [
[
Expand All @@ -838,33 +843,37 @@ def set_network(

# set climb phase
i_initial = 1
if climbing_ratio != 0 :
if climbing_ratio != 0:
dist = 0
alt = p0.height
while dist < climb_dist and i_initial != nb_forward_points:

local_dist = (
pt[i_initial - 1][half_lateral_points][half_vertical_points].distanceTo(
p1
)
pt[i_initial - 1][half_lateral_points][
half_vertical_points
].distanceTo(p1)
) / (nb_forward_points - i_initial)
dist += local_dist
alt += int(local_dist * climbing_ratio)

for k in range(nb_vertical_points):
bearing = pt[i_initial - 1][half_lateral_points][k].initialBearingTo(p1)
bearing = pt[i_initial - 1][half_lateral_points][
k
].initialBearingTo(p1)
pt[i_initial][half_lateral_points][k] = pt[i_initial - 1][
half_lateral_points
][k].destination(local_dist, bearing, min(possible_altitudes[0], alt))
][k].destination(
local_dist, bearing, min(possible_altitudes[0], alt)
)
i_initial += 1

# set last step, descent
i_final = 1
if descending_ratio != 0 :
if descending_ratio != 0:
dist = 0
alt = p1.height

while dist < descent_dist and i_final != nb_forward_points-1:
while dist < descent_dist and i_final != nb_forward_points - 1:
local_dist = (
pt[nb_forward_points - i_final][half_lateral_points][
half_vertical_points
Expand Down Expand Up @@ -905,20 +914,18 @@ def set_network(
pt[half_forward_points][nb_lateral_points - 1][k] = pt[half_forward_points][
half_lateral_points
][k].destination(
distp * half_lateral_points, bearing + 90, height=pt[half_forward_points][
half_lateral_points
][k].height
distp * half_lateral_points,
bearing + 90,
height=pt[half_forward_points][half_lateral_points][k].height,
)
pt[half_forward_points][0][k] = pt[half_forward_points][
half_lateral_points
][k].destination(
distp * half_lateral_points, bearing - 90, height=pt[half_forward_points][
half_lateral_points
][k].height
distp * half_lateral_points,
bearing - 90,
height=pt[half_forward_points][half_lateral_points][k].height,
)



for j in range(1, half_lateral_points + 1):
for k in range(len(possible_altitudes)):
# +j (left)
Expand Down Expand Up @@ -1024,7 +1031,9 @@ def set_network(
][half_lateral_points + j][k].destination(
total_distance / (half_forward_points - i + 1),
bearing,
height=pt[half_forward_points + i][half_lateral_points][k].height,
height=pt[half_forward_points + i][half_lateral_points][
k
].height,
)

bearing = pt[half_forward_points + i - 1][half_lateral_points - j][
Expand All @@ -1042,7 +1051,9 @@ def set_network(
][half_lateral_points - j][k].destination(
total_distance / (half_forward_points - i + 1),
bearing,
height=pt[half_forward_points + i][half_lateral_points][k].height,
height=pt[half_forward_points + i][half_lateral_points][
k
].height,
)
for i in range(abs(half_forward_points - i_final), half_forward_points):
alt = pt[half_forward_points + i - 1][half_lateral_points][k].height
Expand Down
2 changes: 1 addition & 1 deletion skdecide/hub/domain/up/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

from .up import UPDomain, SkUPState, SkUPAction
from .up import SkUPAction, SkUPState, UPDomain
31 changes: 15 additions & 16 deletions skdecide/hub/domain/up/up.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,29 @@
# LICENSE file in the root directory of this source tree.

from typing import Dict, List, Optional, Tuple, Union
import gym

import gym
import unified_planning as up
from unified_planning.model.state import UPState

from skdecide.core import ImplicitSpace, Space, Value
from skdecide.domains import DeterministicPlanningDomain
from skdecide.hub.space.gym import ListSpace, DictSpace
from skdecide.utils import logger

from unified_planning.model import Problem, UPState, InstantaneousAction, FNode
from unified_planning.plans import ActionInstance
from unified_planning.engines.compilers.grounder import GrounderHelper
from unified_planning.engines.sequential_simulator import (
UPSequentialSimulator,
evaluate_quality_metric,
)
from unified_planning.exceptions import UPValueError
from unified_planning.model import FNode, InstantaneousAction, Problem, UPState
from unified_planning.model.metrics import (
MaximizeExpressionOnFinalState,
Oversubscription,
TemporalOversubscription,
)
from unified_planning.model.state import UPState
from unified_planning.plans import ActionInstance
from unified_planning.shortcuts import FluentExp
from unified_planning.engines.sequential_simulator import (
UPSequentialSimulator,
evaluate_quality_metric,
)
from unified_planning.engines.compilers.grounder import GrounderHelper
from unified_planning.exceptions import UPValueError

from skdecide.core import ImplicitSpace, Space, Value
from skdecide.domains import DeterministicPlanningDomain
from skdecide.hub.space.gym import DictSpace, ListSpace
from skdecide.utils import logger


class SkUPState:
Expand Down
6 changes: 3 additions & 3 deletions skdecide/hub/solver/up/up.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@

from __future__ import annotations

from typing import Any, Callable, Dict, List

from unified_planning.engines import Engine
from unified_planning.exceptions import UPValueError
from unified_planning.shortcuts import FluentExp, SequentialSimulator

from skdecide.hub.domain.up import UPDomain, SkUPState, SkUPAction
from typing import Any, Callable, Dict, List

from skdecide import Solver, Value
from skdecide.builders.solver import DeterministicPolicies, Utilities
from skdecide.hub.domain.up import SkUPAction, SkUPState, UPDomain


# TODO: remove Markovian req?
Expand Down
8 changes: 4 additions & 4 deletions tests/domains/python/test_up_bridge_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,23 @@
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

import pytest
import sys

import gym
import pytest


@pytest.mark.skipif(sys.version_info < (3, 10), reason="requires python3.10 or higher")
def test_up_bridge_domain():
noexcept = True

try:
from skdecide.hub.domain.up import UPDomain
from skdecide.hub.solver.lazy_astar import LazyAstar

import unified_planning
from unified_planning.shortcuts import Fluent, InstantaneousAction, Not

from skdecide.hub.domain.up import UPDomain
from skdecide.hub.solver.lazy_astar import LazyAstar

x = Fluent("x")
y = Fluent("y")

Expand Down
Loading

0 comments on commit 1ba371e

Please sign in to comment.