PowerGridModel / power-grid-model-io

Conversion tool for various grid data formats to power-grid-model
Mozilla Public License 2.0
12 stars 8 forks source link

Pandapower Jupyter example notebook log output fix #221

Closed Jerry-Jinfeng-Guo closed 7 months ago

Jerry-Jinfeng-Guo commented 7 months ago

The problem relates to the Pandas library future warnings. To properly address the issue, we need to migrate current code take advantage of new calls before Pandas officially deprecates the problematic calls.

[Details] The following 2 type of future warnings are given (and suppressed):

mgovers commented 7 months ago

warning suppression

There is at least one offending line that is directly caused by a pandapower call: https://github.com/PowerGridModel/power-grid-model-io/blob/main/docs/examples/pandapower_example.ipynb?short_path=77c97a3#L55 . This argues that suppressing the FutureWarning for pandas is fine

FutureWarning: Setting an item of incompatible dtype is deprecated and will raise an error in a future version of pandas. Value 'Dyn' has dtype incompatible with float64, please explicitly cast to a compatible dtype first.
  net[element_type].at[index, column] = value

pandas version fixing

fillna

Then there is another issue that is in our code base and is quite easily fixed https://github.com/PowerGridModel/power-grid-model-io/blob/main/src/power_grid_model_io/converters/pandapower_converter.py#L2258 :

FutureWarning: Downcasting object dtype arrays on .fillna, .ffill, .bfill is deprecated and will change in a future version. Call result.infer_objects(copy=False) instead. To opt-in to the future behavior, set `pd.set_option('future.no_silent_downcasting', True)`

We only need to verify whether the new behavior or the old behavior is preferred. They even provide a hint towards the alternative

There is one more location where fillna is used in https://github.com/PowerGridModel/power-grid-model-io/blob/main/src/power_grid_model_io/converters/pandapower_converter.py#L2258 and I don't know without further investigation whether that provides any issues

Chained Assignment

The chained assignment error in e.g. https://github.com/PowerGridModel/power-grid-model-io/blob/main/src/power_grid_model_io/converters/pandapower_converter.py#L841 is indeed a more difficult issue.

My expectation is that instead of ["A"]["B"] we should just be able to do .loc["B", "A"] as mentioned in the error message, but that's an assumption that needs to be verified

FutureWarning: ChainedAssignmentError: behaviour will change in pandas 3.0!
You are setting values through chained assignment. Currently this works in certain cases, but when using Copy-on-Write (which will become the default behaviour in pandas 3.0) this will never work to update the original DataFrame or Series, because the intermediate object on which we are setting values will behave as a copy.
A typical example is when you are setting values in a column of a DataFrame, like:

df["col"][row_indexer] = value

Use `df.loc[row_indexer, "col"] = values` instead, to perform the assignment in a single step and ensure this keeps updating the original `df`.

See the caveats in the documentation: https://pandas.pydata.org/pandas-docs/stable/user_guide/indexing.html#returning-a-view-versus-a-copy

Conclusion

If you - instead of version fixing pandas - solve these issues, we should be good to go

sonarcloud[bot] commented 7 months ago

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Jerry-Jinfeng-Guo commented 7 months ago

Currently the example notebook still contains the warning suppressing code because Pandapower itself raises some warning.