Refactor results writers to support dual access grids #954
Conversation
The `MultiGridResultWriter` and `GridResultWriter` can be reused for dual access grids with minor changes. Regular accessibility grids use time cutoffs, dual accessibility grids use thresholds. This change mainly moves up the `channels` parameter to support cutoffs and thresholds (channels, cutoffs, thresholds...naming here could be confusing). This change additionally does some minor refactoring around passing the file name to `BaseResultWriter`'s constructor so that metadata doesn't need to be passed and stored until `finish()`.
| /** | ||
| * We create one GridResultWriter for each destination pointset and percentile. | ||
| * Each of those output files contains data for all travel time cutoffs at each origin. | ||
| * Each of those output files contains data for all channels at each origin. |
There was a problem hiding this comment.
The term "channels" is understandable by analogy with image files, but it threw me off at first. We are using "thresholds" and "travel time cutoffs" as mutually exclusive categories and introduce the word "channels" to refer to the slots for either of them in the file. But actually a time cutoff is a kind of threshold - I often use the expression "travel time threshold" interchangeably with "travel time cutoff". So, suggestion for simplifying terminology:
| * Each of those output files contains data for all channels at each origin. | |
| * Each of those output files contains data for all thresholds (time or cumulative access) at each origin. |
I think this would get rid of the word channels entirely, and we could just call use thresholds and nThresholds everywhere. Then in specific places we could say "time threshold" or "(cumulative) access threshold".
| @@ -39,7 +37,7 @@ public abstract class CsvResultWriter extends BaseResultWriter implements Region | |||
| * Override to return an Enum (usable as String) identifying the kind of results recorded by this CSV writer. | |||
| * This serves as a filename suffix to distinguish between different CSVs generated by a single regional analysis. | |||
| */ | |||
| public abstract CsvResultType resultType (); | |||
| public final CsvResultType resultType; | |||
There was a problem hiding this comment.
The behavior and characteristics that are common to all instances of each subclass were being specified by overriding methods. This final field should work fine in place of the method, but I was wondering if there was some reason for the change.
Refactor the
MultiGridResultWriterandGridResultWriterso they can be reused for dual access grids with minor additional changes.Regular accessibility grids use time cutoffs, dual accessibility grids use thresholds. This change mainly moves up the
channelsparameter to support cutoffs and thresholds (channels, cutoffs, thresholds...naming here could be confusing).This change additionally does some minor refactoring around passing the file name to
BaseResultWriter's constructor so that metadata doesn't need to be passed and stored untilfinish().