USEPA / WNTR

An EPANET compatible python package to simulate and analyze water distribution networks under disaster scenarios.
Other
317 stars 183 forks source link

'node_type' is used when assigning nodes to junction, tank, reservoir when importing from gis rather than which gdf they are in #448

Open angusmcb opened 1 month ago

angusmcb commented 1 month ago

Summary Could be a bug or a feature.

When importing from a water network from_gis, the column 'node_type' (and 'link_type') is used to decide what type of node it is. Whist useful, this is an undocumented feature/bug and I think that normal behaviour would be that it is decided by whether it is in the 'junctions', 'tanks','reservoirs' dataframe instead.

In fact, 'node_type' is not listed as one of the valid column names.

Example I tweaked one of the tests from test_network.py:

    def test_gis_roundtrip(self):
        for inp_file in self.inp_files:
            wn = self.wntr.network.WaterNetworkModel(inp_file)
            A = wn.to_gis()
            A.junctions.replace('Junction','Reservoir',inplace=True) # Should make no difference
            B = self.wntr.network.from_gis(A)
            assert(wn._compare(B, level=0))

Expected behaviour is no change - test should pass - because junctions would be detected as being junctions from the fact they are in the 'junctions' dataframe, rather than a property within the dataframe. (I realise this is really not the best example ).

Environment Provide information on your computing environment.

kaklise commented 4 weeks ago

Thanks for bringing this to our attention, this is unintended behavior. node_type and link_type are used in the dictionary representation of a water network model (wn.to_dict, wn.from_dict), which combines junctions/reservoirs/tanks into nodes and pipes/pumps/valves into links. The dictionary representation is also used to convert water network models to/from GeoDataFrames (to_gis calls to_dict and from_gis calls from_dict). However, as you point out the object type should is known and should not change. We discussed an easy fix that will ensure, for example, the information in A.junctions will always be Junctions.