Gurobi / gurobipy-pandas

Convenience wrapper for building optimization models from pandas data
https://gurobipy-pandas.readthedocs.io
Apache License 2.0
90 stars 15 forks source link

Series accessor should do type verification #12

Closed rluce closed 1 year ago

rluce commented 2 years ago

Pandas example includes a validator method at construction time: https://pandas.pydata.org/docs/development/extending.html

This is probably only sensible for our purposes on the Series accessor. The constructor should check:

  1. (at least) the series is dtype=object, or a gurobipy extension type if we implement that.
  2. (maybe) if dtype=object, then all objects must be gurobipy objects of the same type.

By way of comparison, the StringMethods accessor checks only the dtype. If the final called method fails on some objects in the series, they are left as NaN in the result.

>> pd.Series([0, 1, 2]).str  # dtype: int
AttributeError: Can only use .str accessor with string values!

>> pd.Series([0, 'a', 'b']).str.upper()
0    NaN
1      A
2      B
dtype: object

We may not need to check (2) at construction time but I think it makes more sense in our case to fail if the required operation fails on any object in the series.

Dr-Irv commented 2 years ago

Another thing to consider is to provide typing stubs for the accessors. We're now publishing pandas-stubs, and the way that Series.str is hooked in is via a StringAccessor. Problem for you is that you can't add an accessor to the stub. But I think there is a way for you to do this.

If the type of something like Series.grb.pd_add_vars() became GRBVarSeries, where GRBVarSeries is a subclass of Series, you could then add a typing stub for GRBVarSeries.grb, and then type check the various methods on the accessors.

MIght be nice to have GRBVarSeries (and maybe GRBConstrSeries, GRBLinExprSeries, etc.) so you know the type of the series via its naming convention.

Could also consider making a Generic type of GRBSeries[gurobipy.Var] . Note that the pandas-stubs are using Generic on Series to make the typing work better.

simonbowly commented 1 year ago

Copied @Dr-Irv's comment above to #41 where we discuss type hints. This issue was to cover checking that the objects held by a series are of a sensible type when the accessor is called.

simonbowly commented 1 year ago

We won't do any validation for now. The series accessor also allows for variable creation, so we cannot assume the user is going to call down to gurobi attributes.