Conversation
|
all tests are passed, I refactored a lot, and used the pytest -vv to spot errors |
MartinBruun
left a comment
MartinBruun
left a comment
There was a problem hiding this comment.
Koden er fuldt ud accepteret, men der er lige noget i testene jeg synes virker lidt mærkeligt.
src/__pytest__/conftest.py
Outdated
| "highway": "residential", | ||
| "maxspeed": 80, | ||
| "edge_adj": 2, | ||
| "edge_adj": 3, |
There was a problem hiding this comment.
Denne forstår jeg ikke?
1 -> 3 gør jo at det ikke længere er en "straight line" som denne fixture i dens beskrivelse siger den er?
Ie.
Node 0 -> Node 1 -> Node 2 -> Node 3
(og modsat vej, givende 3*2 edges)
Hvis du mener at du prøver at modellere noget andet, så tænker jeg at beskrivelsen af denne "fixture collection" bør opdateres til hvad du mener det repræsentere :)
src/__pytest__/conftest.py
Outdated
| "edge_adj": 6, | ||
| }, | ||
| { | ||
| "edge_id": 6, |
There was a problem hiding this comment.
Hvorfor er der behov for en 7. edge?
Tænker det vil være bedre at lave en ny "fixture collection" som modellere et mere sammenhængende netværk (som ser ud til at være det du ønsker?) Men at vi stadig skal have et "bare bones" / simpelt "straight line" netværk?
| { | ||
| "to_node_id": 0, | ||
| "weight": 1, | ||
| "edge_id": 0, |
There was a problem hiding this comment.
Fjern edge_id, medmindre der er et rigtigt godt argument for at beholde det.
src/__pytest__/conftest.py
Outdated
| "to_node_id": 1, | ||
| "weight": 1, | ||
| "edge_id": 3, | ||
| "to_node_id": 4, |
There was a problem hiding this comment.
Denne giver ikke mening, da vi jo kun har node 0, 1, 2 og 3.
src/__pytest__/conftest.py
Outdated
| "to_node_id": 3, | ||
| "weight": 1, | ||
| "edge_id": 4, | ||
| "to_node_id": 5, |
There was a problem hiding this comment.
Denne giver ikke mening, da vi jo kun har node 0, 1, 2 og 3.
src/__pytest__/conftest.py
Outdated
| "to_node_id": 2, | ||
| "weight": 1, | ||
| "edge_id": 5, | ||
| "to_node_id": 6, |
There was a problem hiding this comment.
Denne giver ikke mening, da vi jo kun har node 0, 1, 2 og 3.
| #nodesJSON = table_kwargs["nodes"] | ||
| # dataModel.errorList = table_kwargs["errors"] | ||
| # dataModel.metadata = table_kwargs["metadata"] | ||
| # dataModel.vehicle = table_kwargs["vehicle"] |
| for row in table_kwargs["nodes"]: | ||
| node=makeNode(row) | ||
| dataModel.nodes.insert(node.node_id, node)#add node at its node_id | ||
| dataModel["nodes"].insert(node["node_id"], node)#add node at its node_id |
There was a problem hiding this comment.
Som udgangspunkt er jeg nok lettere uenig i at sætte kode kommentarer for hvad koden gør.
Tror grunden til vi gør det nu, er fordi python jo er et nyt sprog, så vi kan godt lade det stå, hvis det er.
|
Finno |
2 participants
No description provided.