fix: correct type of new bus index in Grid.bus2sub

[danielolsen]

Purpose

Fix a bug, introduced in #352, for the indexing of the bus2sub table when calling scenario.state.get_grid() on a Scenario in Create state with one or more new buses.

What the code is doing

We fix new_bus_id so that it is an int, not a list, and wrap this int into a new list when appending to the bus dataframe. This avoids a 'double-listing' that creates a tuple as the new index in the bus2sub table.

Testing

Previously:

>>> from powersimdata.scenario.scenario import Scenario
>>> scenario = Scenario('')
>>> scenario.state.set_builder(["Texas"])
[...truncated...]
>>> scenario.state.builder.change_table.add_bus([{"lat": 30, "lon": -95, "zone_id": 308, "Pd": 0, "baseKV": 230}])
>>> scenario.state.get_grid().bus2sub
            sub_id interconnect
3001001      39762        Texas
3001002      39763        Texas
3001003      39764        Texas
3001004      39764        Texas
3001005      39765        Texas
...            ...          ...
3008157      41010        Texas
3008158      41011        Texas
3008159      41011        Texas
3008160      41011        Texas
(3008161,)   41012        Texas

[2001 rows x 2 columns]
>>> scenario.state.get_grid().bus
         type     Pd     Qd  Gs  ...  mu_Vmin  interconnect      lat       lon
3001001     1  20.78   5.89   0  ...        0         Texas  31.9067 -102.2620
3001002     1  15.41   4.37   0  ...        0         Texas  29.8880 -104.5190
3001003     1   0.00   0.00   0  ...        0         Texas  32.9264 -101.6480
3001004     2   0.00   0.00   0  ...        0         Texas  32.9264 -101.6480
3001005     1   0.00   0.00   0  ...        0         Texas  32.2075 -101.3880
...       ...    ...    ...  ..  ...      ...           ...      ...       ...
3008157     1   0.00   0.00   0  ...        0         Texas  31.0919  -96.6950
3008158     2   0.00   0.00   0  ...        0         Texas  30.7217  -96.4608
3008159     1   0.00   0.00   0  ...        0         Texas  30.7217  -96.4608
3008160     1  90.76  25.71   0  ...        0         Texas  30.7217  -96.4608
3008161     1   0.00   0.00   0  ...        0         Texas  30.0000  -95.0000

[2001 rows x 19 columns]

After the fix:

>>> from powersimdata.scenario.scenario import Scenario
>>> scenario = Scenario('')
>>> scenario.state.set_builder(["Texas"])
[...truncated...]
>>> new_scenario.state.builder.change_table.add_bus([{"lat": 30, "lon": -95, "zone_id": 308, "Pd": 0, "baseKV": 230}])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'new_scenario' is not defined
>>> scenario.state.builder.change_table.add_bus([{"lat": 30, "lon": -95, "zone_id": 308, "Pd": 0, "baseKV": 230}])
>>> scenario.state.get_grid().bus2sub
         sub_id interconnect
3001001   39762        Texas
3001002   39763        Texas
3001003   39764        Texas
3001004   39764        Texas
3001005   39765        Texas
...         ...          ...
3008157   41010        Texas
3008158   41011        Texas
3008159   41011        Texas
3008160   41011        Texas
3008161   41012        Texas

[2001 rows x 2 columns]
>>> scenario.state.get_grid().bus
         type     Pd     Qd  Gs  ...  mu_Vmin  interconnect      lat       lon
3001001     1  20.78   5.89   0  ...        0         Texas  31.9067 -102.2620
3001002     1  15.41   4.37   0  ...        0         Texas  29.8880 -104.5190
3001003     1   0.00   0.00   0  ...        0         Texas  32.9264 -101.6480
3001004     2   0.00   0.00   0  ...        0         Texas  32.9264 -101.6480
3001005     1   0.00   0.00   0  ...        0         Texas  32.2075 -101.3880
...       ...    ...    ...  ..  ...      ...           ...      ...       ...
3008157     1   0.00   0.00   0  ...        0         Texas  31.0919  -96.6950
3008158     2   0.00   0.00   0  ...        0         Texas  30.7217  -96.4608
3008159     1   0.00   0.00   0  ...        0         Texas  30.7217  -96.4608
3008160     1  90.76  25.71   0  ...        0         Texas  30.7217  -96.4608
3008161     1   0.00   0.00   0  ...        0         Texas  30.0000  -95.0000

[2001 rows x 19 columns]

Time estimate

2 minutes.

[rouille]

I guess this bug could have also been fixed by removing the bracket to the new_bus_id l.259 and l.270 where you set the index in the DataFrame.

[danielolsen]

I guess this bug could have been fixed by removing the bracket to the new_bus_id l.259 and l.270 where you set the index in the DataFrame.

Yes, that would be equivalent. Based on the naming new_bus_id, I think it is more clear if this is the id of the new bus, rather than a list containing this value. If we changed l.259 & l.270, I think we would want to rename this variable to something like new_bus_index.

[rouille]

I guess this bug could have been fixed by removing the bracket to the new_bus_id l.259 and l.270 where you set the index in the DataFrame.

Yes, that would be equivalent. Based on the naming new_bus_id, I think it is more clear if this is the id of the new bus, rather than a list containing this value. If we changed l.259 & l.270, I think we would want to rename this variable to something like new_bus_index.

Yeah, we call this variable new_index in _add_dcline for example.

[rouille]

Looks good

[danielolsen]

I guess this bug could have been fixed by removing the bracket to the new_bus_id l.259 and l.270 where you set the index in the DataFrame.

Yes, that would be equivalent. Based on the naming new_bus_id, I think it is more clear if this is the id of the new bus, rather than a list containing this value. If we changed l.259 & l.270, I think we would want to rename this variable to something like new_bus_index.

Yeah, we call this variable new_index in _add_dcline for example.

Do you think I should rename to match?

[rouille]

I guess this bug could have been fixed by removing the bracket to the new_bus_id l.259 and l.270 where you set the index in the DataFrame.

Yes, that would be equivalent. Based on the naming new_bus_id, I think it is more clear if this is the id of the new bus, rather than a list containing this value. If we changed l.259 & l.270, I think we would want to rename this variable to something like new_bus_index.

Yeah, we call this variable new_index in _add_dcline for example.

Do you think I should rename to match?

Do you read my mind? It is up to you.

[BainanXia]

It looks good to me. I vote for being consistent in naming variables though :)

[danielolsen]

Variables are tweaked, and a test is added that would catch the issue if there's a regression.