Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type hint and validate Instance and Solution dictionaries #100

Closed
wants to merge 13 commits into from
Prev Previous commit
Next Next commit
Ignore variable as key in TypedDict
leonlan committed May 2, 2023
commit a8a16a633631f6c2a312ce3077c84c948503fb58
11 changes: 7 additions & 4 deletions vrplib/parse/parse_solution.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from typing import Dict, List, Union
from typing import List, TypedDict

from .parse_utils import infer_type, text2lines

Solution = Dict[str, Union[float, str, List]]

class Solution(TypedDict, total=False):
routes: List[List[int]]
cost: float


def parse_solution(text: str) -> Solution:
@@ -25,11 +28,11 @@ def parse_solution(text: str) -> Solution:
for line in text2lines(text):
if "Route" in line:
route = [int(idx) for idx in line.split(":")[1].split(" ") if idx]
solution["routes"].append(route) # type: ignore
solution["routes"].append(route)
elif ":" in line or " " in line: # Split at first colon or whitespace
split_at = ":" if ":" in line else " "
k, v = [word.strip() for word in line.split(split_at, 1)]
solution[k.lower()] = infer_type(v)
solution[k] = infer_type(v) # type: ignore
else: # Ignore lines without keyword-value pairs
continue

4 changes: 2 additions & 2 deletions vrplib/parse/parse_vrplib.py
Original file line number Diff line number Diff line change
@@ -38,11 +38,11 @@ def parse_vrplib(text: str, compute_edge_weights: bool = True) -> Instance:

for spec in specs:
key, value = parse_specification(spec)
instance[key] = value
instance[key] = value # type: ignore

for section in sections:
section, data = parse_section(section, instance)
instance[section] = data
instance[section] = data # type: ignore
Copy link
Member Author

@leonlan leonlan May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypedDict requires string literals, but since I don't know what kinds of section are parsed I need to ignore this.

I'm not sure if it even makes sense to use a TypedDict if I ignore this line.

Copy link
Member Author

@leonlan leonlan May 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's OK. Even though this line is no longer checked, the returned instance is assumed to be of type Instance, which used by mypy to do static type checking.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if we do this, we also need to validate that the instance indeed follows the types as specified in Instance.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can use pydantic for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make a new validate module that validates the instance and solution outputs according to VRPLIB.


if instance and compute_edge_weights and "edge_weight" not in instance:
# Compute edge weights if there was no explicit edge weight section