Transport-for-the-North / caf.core

Core classes and definitions for CAF family of tools
GNU General Public License v3.0
0 stars 1 forks source link

DVector.data setter no longer functions correctly #39

Closed asongtoruin closed 1 month ago

asongtoruin commented 1 month ago

DVector._dataframe_to_dvec has been changed to return a tuple of (data, segmentation):

https://github.com/Transport-for-the-North/caf.core/blob/4e2a88684b03691d0b25a5225d81a472d078db98/src/caf/core/data_structures.py#L448

However, the DVector.data setter method has not been updated in line with this, and as such using the setter will result in the .data property being a tuple rather than a DataFrame:

https://github.com/Transport-for-the-North/caf.core/blob/4e2a88684b03691d0b25a5225d81a472d078db98/src/caf/core/data_structures.py#L322-L331

A very quick solution for this would be to instead set self._data, _ = self._dataframe_to_dvec(value). More generally, adding return type hints to the DVector methods might have resulted in this being flagged by the linter. @isaac-tfn could you or Matteo (who I can't tag) take a quick look?

isaac-tfn commented 1 month ago

I'll take a look now thanks

isaac-tfn commented 1 month ago

OK gone for the minimal fix for now, should be working.