Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#2636 closed defect (fixed)

ogr_mitab.py test #9 fails

Reported by: aboudreault Owned by: Daniel Morissette
Priority: normal Milestone: 1.6.0
Component: default Version: unspecified
Severity: normal Keywords:
Cc: warmerdam

Description

See related bug: Mitab bug 1945

After this fix, the test #9 of ogr_mitab.py fails. The data used by the test are the files small.mif and small.mid. small.mid includes a string with a quote (") inside it to test. But this file use the old way to store quote in a string (\"). The small.mid file must be modified to use the same way as MapInfo? does.

By checking this test fails, i found another error in the small.mif. It includes a space after each comma. This is a bad case for MapInfo?. When MapInfo? reads the first character of a character field and it is not a quote ("), it thinks that the character field doesn't have string delimiter. So, a string:

field1, "Guarino Chucky Sandra",field3 (real string: Guarino Chucky Sandra) become (real string: "Guarino Chucky Sandra")

The Mitab/trunk and MapInfo?(9) doesn't includes space after comma. i suppose that this file has been manually modified ? We don't know the real origin of the file small.mid.

Finally, to have a good test case for Mitab and be MapInfo? compatible, the files small.mid and small_ntf.mid must have these modifications:

  1. Replace all '\"' by '""'.
  2. Remove all space after comma.

Attachments (1)

bug2636.patch (946 bytes) - added by aboudreault 12 years ago.
Here's the patch... Daniel could you test it plz, i don't have the setup here to test gdal suite.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 12 years ago by aboudreault

Cc: Daniel Morissette added

comment:2 Changed 12 years ago by Daniel Morissette

Cc: warmerdam added; Daniel Morissette removed
Milestone: 1.6.0
Owner: changed from warmerdam to Daniel Morissette
Status: newassigned

Alan, please attach an updated small.mid that works and I'll commit it to the GDAL SVN.

Changed 12 years ago by aboudreault

Attachment: bug2636.patch added

Here's the patch... Daniel could you test it plz, i don't have the setup here to test gdal suite.

comment:3 Changed 12 years ago by Even Rouault

I updated and recompiled my trunk, applied bug2636.patch, but it is still failing on ogr_mitab_9, with a different error message.

ogrinfo -ro -al data/small.mif gives :

INFO: Open of `data/small.mif'
      using driver `MapInfo File' successful.

Layer name: small
Geometry: Unknown (any)
Feature Count: 2
Extent: (407128.974000, 155312.559000) - (407142.741000, 155329.616000)
Layer SRS WKT:
LOCAL_CS["Nonearth",
    UNIT["Meter",1]]
NAME: String (50.0)
BLOCK: Integer (0.0)
OWNER: String (64.0)
APPRAISED_VALUE: Real (0.0)
TAXES: Real (0.0)
FLOODZONE: Integer (0.0)
NUM: String (254.0)
ERROR 6: Error during reading Record.

comment:4 Changed 12 years ago by Daniel Morissette

I had a look and it turns out that the small.mid header specifies the delimiter as follows:

Delimiter ", "

That would explain why we used to have spaces in the .mid file delimiters and also why MITAB produces an error with the new file.

Now I wonder why MapInfo? failed in Alan's tests and if multi-chars delimiters are valid or not. We'll need to make more tests in MapInfo? tomorrow, but I don't think that should block the 1.6.0 beta tonight.

In the meantime I have committed in r15644 a new version of the .mid files with the double quotes fix but without the change to the delimiter. That should let test #9 pass, I'll verify that with the buildbot after my commit since I'm not setup to run the testsuite locally yet.

comment:5 Changed 12 years ago by Even Rouault

Resolution: fixed
Status: assignedclosed

Yes, ogr_mitab_9 passes now

comment:6 Changed 12 years ago by Daniel Morissette

Confirmed with the buildbot that ogr_mitab_9 passes.

We still need to figure out what was the real source of the problem with the ", " delimiter and if that is a valid case in MIF files or not...

Actually, looking at the MIF docs at http://www.directionsmag.com/mapinfo-l/mif/AppJ.pdf I think we got our answer, the delimiter must be a single character:

Delimiter

Specify the delimiting character in quotation marks, for example:

  DELIMITER ”;”

The default delimiter is Tab; if you are using the default, you do not need the
DELIMITER line.

In a few other places in the document we also refer to the delimiter as a single character, so I think that rules out the possibility of multi-char delimiters, and even if MITAB accepts and supports that case, MapInfo? itself doesn't and I don't think it's a good idea to have that in our testsuite.

Based on this I have committed r15645 that removes the spaces in the delimiter in the MIF header and in the MID files.

comment:7 Changed 12 years ago by Daniel Morissette

Um... in http://bugzilla.maptools.org/show_bug.cgi?id=1266 we have contradictory information... apparently the reporter had verified that a ", " delimiter worked in MapInfo? and that led Frank to implementing the support for multichar delimiters as it is today.

Note: See TracTickets for help on using tickets.