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

Custom cache path not properly echoed through ZoningSystem.translate #26

Closed asongtoruin closed 3 months ago

asongtoruin commented 4 months ago

We've been trying to set up some translations with a non-default cache path, but after a certain point in the chain the custom cache path stops being included, and as such the output location resets to the default.

To follow the chain through, ZoningSystem.translate accepts the cache path and passes it through to ZoningSystem._get_translation_definition. However, when this subsequently calls ZoningSystem._generate_spatial_translation the cache path is not passed through:

https://github.com/Transport-for-the-North/caf.core/blob/a644f6f719232e9ac2d0eefb289f591153b3385b/src/caf/core/zoning.py#L432

This in turn ultimately sets up ZoningTranslationInputs (from caf.space):

https://github.com/Transport-for-the-North/caf.core/blob/a644f6f719232e9ac2d0eefb289f591153b3385b/src/caf/core/zoning.py#L358-L383

which can accept a cache_path.

I think there are two steps to fix:

  1. Update _generate_spatial_translation to take cache_path as a parameter and pass it through to ZoningTranslationInputs
  2. Update _get_translation_definition so that it passes the cache_path through to it.
isaac-tfn commented 3 months ago

As with the other issue, sorry I didn't spot that this has been raised, I'll look at fixing it tomorrow

asongtoruin commented 3 months ago

This issue appears to still not be fixed, and actually is now causing different issues. Line 447 still does not pass through the cached path:

https://github.com/asongtoruin/caf.core/blob/9bdb3f8d0b2f84da11c9f0cb67f12ebb4d89784d/src/caf/core/zoning.py#L447

The default cache path for _generate_spatial_translation also looks incorrect:

https://github.com/asongtoruin/caf.core/blob/9bdb3f8d0b2f84da11c9f0cb67f12ebb4d89784d/src/caf/core/zoning.py#L370-L372

Should this not be ZONE_TRANSLATION_CACHE? @isaac-tfn can you review please?

isaac-tfn commented 3 months ago

Hi Adam, yes you're right on both counts, should be fixed now sorry