DeepForest icon indicating copy to clipboard operation
DeepForest copied to clipboard

Root dir is just an extra annoying argument for specifying training and evaluation.

Open bw4sz opened this issue 2 years ago • 3 comments

The API currently formats input csv files to have a image_path filename that is relative to a separately entered root dir. If I reach back in time, I think this was because this was the same format as keras-retinanet. I no longer find this reasonable. Why not have image_path be the full path?

Let's consider the pros and cons of the current system of relative paths

pros

  1. Allows users to images to move images without needing to remake the csv file.
  2. ?

Cons

  1. Forces a single root dir, meaning that all images need to be in some massive directory. Why is this required, why is deepforest insisting that the user conform to a preset image directory structure? The root dir and image_path's are always merged. For example
  2. Additional arguments that feels non-pythonic.
image

The major downside is that this is a breaking change. @ethanwhite I might need some opinions about how we might create a 2.0 branch that we merge into instead of main? Or if we go to DeepForest 1.3 and call it good?

bw4sz avatar Oct 11 '23 17:10 bw4sz

Here too. It has no use.

def boxes_to_shapefile(df, root_dir, projected=True, flip_y_axis=False):
    """
    Convert from image coordinates to geographic coordinates
    Note that this assumes df is just a single plot being passed to this function
    Args:
       df: a pandas type dataframe with columns: name, xmin, ymin, xmax, ymax. Name is the relative path to the root_dir arg.
       root_dir: directory of images to lookup image_path column
       projected: If True, convert from image to geographic coordinates, if False, keep in image coordinate system
       flip_y_axis: If True, reflect predictions over y axis to align with raster data in QGIS, which uses a negative y origin compared to numpy. See https://gis.stackexchange.com/questions/306684/why-does-qgis-use-negative-y-spacing-in-the-default-raster-geotransform
    Returns:
       df: a geospatial dataframe with the boxes optionally transformed to the target crs
    """

bw4sz avatar Oct 11 '23 22:10 bw4sz

Is it possible to add a default to the argument that changes the behavior while maintaining backward compatibility? E.g., in this boxes_to_shapefile case we could set root_dir = None as the default which would result in the new behavior, but old code that included an argument to second position would still work as intended.

ethanwhite avatar Oct 11 '23 23:10 ethanwhite

Add deprecation here until 2.0

bw4sz avatar Oct 16 '23 19:10 bw4sz