search

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._generic_dunder has unrunnable conditions (rewrite_exclusions branch) #35

Closed asongtoruin closed 1 month ago

asongtoruin commented 2 months ago

Local variable out is only set under certain circumstances (i.e. when the segmentation of two DVector objects don't match):

https://github.com/Transport-for-the-North/caf.core/blob/f2947fd194c9bfc5db4134bcbff35e14eb8bce0e/src/caf/core/data_structures.py#L632-L636

However, this variable is always references when we have low_memory=False, i.e. the default behaviour for the class, and either matching zone systems or a missing zoning system for either object:

https://github.com/Transport-for-the-North/caf.core/blob/f2947fd194c9bfc5db4134bcbff35e14eb8bce0e/src/caf/core/data_structures.py#L669-L695

@isaac-tfn, can you take a look? Could it be as simple as changing that first snippet to:

if self.segmentation != other.segmentation:
    out = self.expand_to_other(other)
else:
    out = self

It's also perhaps worth noting that out doesn't seem to be referenced at all in the low_memory process, which seems odd?

isaac-tfn commented 1 month ago

OK addressed in latest commit, let me know if it works. By the way none of the new dunder method stuff has been included for 'low_memory=True' as of yet, as it seemed that was never really being used. If you are using that option anywhere let me know and I'll look at including it.

@asongtoruin just tagging so you see, I don't know if you automatically get notifications for issues you're active in.

asongtoruin commented 1 month ago

This seems to run now, closing.