Changes between Version 4 and Version 8 of Ticket #5442


Ignore:
Timestamp:
May 10, 2014, 2:40:03 AM (8 years ago)
Author:
fhissel
Comment:

Replying to rouault:

Ok, but now the ogr_selafin.py script fails :

  TEST: ogr_selafin_create_ds ... ERROR 1: It seems no file system object called 'tmp.slf' exists.
Warning 1: Although Selafin data-sources and layers are writable, it is up to the user to avoid writing on two layers at the same time because this could result in data corruption.
success
  TEST: ogr_selafin_create_nodes ... Warning 1: Although Selafin data-sources and layers are writable, it is up to the user to avoid writing on two layers at the same time because this could result in data corruption.
ERROR 3: Error when reading Selafin file

fail
    line 80: unable to create node feature
  TEST: ogr_selafin_create_elements ... Warning 1: Although Selafin data-sources and layers are writable, it is up to the user to avoid writing on two layers at the same time because this could result in data corruption.
fail
    line 144: wrong value of attribute in element layer
  TEST: ogr_selafin_set_field ... Warning 1: Although Selafin data-sources and layers are writable, it is up to the user to avoid writing on two layers at the same time because this could result in data corruption.
ERROR 5: List should have 1 elements
success

Yes, that was a regression of the patch for Windows. It is fixed now. I also checked the Python script with Valgrind and there is no memory leak (at least in the driver...)

The error occurs at line 376 ( if (Selafin::read_integer(poHeader->fp,nLen,true)==0
) of ogrselafinlayer.cpp

Could you remove the warning about "Although Selafin data-sources and layers are writable, it is up to the user to avoid writing on two layers at the same time because this could result in data corruption.". This is a general requirement for any GDAL driver, so no need to spam the user.

OK, done.

When looking at CreateFeature?()/SetFeature?(), I also see that you assume that a feature has a geometry. This is not necessarily the case.

Sorry, I was not aware of this. I added a test at the beginning of those two functions.

I'm also wondering if you needed to implement DeleteField?(), AlterFieldDefn?(), and even SetFeature?() or DeleteFeature?() etc.. They are advanced features only used by popular and general purpose formats that are meant to be work formats (shapefile, postgis, spatialite, etc...). Specialized formats like Selafin don't really need that I guess, and it could be just a read-only / write-only format, and not bother with modifying.

Yes, my purpose was mainly to allow the creation of Selafin files based on other GIS formats. But this may not require all the functions... I will post another patch with DeleteField?, AlterFieldDefn? and DeleteFeature? marked as non implemented. SetFeature? is actually one of the easiest to implement and can be useful for some purposes (changing some values in QGis...) so I will keep it for now.

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #5442 – Description

    v4 v8  
    11Selafin is a format used by some finite element software (mostly [http://www.opentelemac.org Telemac hydraulic model]) to store numerical values at different points or elements and for different time steps.
    22
    3 This driver allows read-write support for Selafin format in OGR.
     3This driver allows read-write support for Selafin 2D format in OGR.