patchGNU Octave - Patches: patch #9101, shapewrite (mapping package):...

 
 

patch #9101: shapewrite (mapping package): bugfixes and test

Submitter:  Martin Kunz <mkunz>
Submitted:  Sat 27 Aug 2016 06:15:13 AM UTC
   
 
Category:  Forge : other Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 20 Nov 2016 07:24:38 AM UTC, comment #6: 

Thanks for letting me know!

Martin

Anonymous
Sat 19 Nov 2016 10:54:32 PM UTC, comment #5: 

Martin,

I've pushed some more fixes of your patch here:
http://hg.code.sf.net/p/octave/mapping/rev/4658e05a503f

Thx

Philip Nienhuis <philipnienhuis>
Group Member
Fri 07 Oct 2016 07:22:21 PM UTC, comment #4: 

Mrtin,
I accepted most of the hunks and pushed them here:
http://hg.code.sf.net/p/octave/mapping/rev/a80588cbbb70

FYI, I kept the Boundingbox parts; they are just code and the values are not written to the shape file anyway.

Thanks very much.

Closing report.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 12 Sep 2016 05:48:03 PM UTC, comment #3: 

Martin,

A couple of questions & remarks, sorry:

Hunk #1
-------
I want to remove all credits in the file, they should be in the commit message & repo. (FYI, J.Heckman is credited in a commit message.)
I need to clean this up in some other function file s as well.

Hunk #2
-------
Why is "fullfile (ph,fnm)" required ? IOW, what goes wrong with the simple "[pth fnm]" ?
If it is required it should apply to the main .shp file too.
AFAICT relative file names do work fine.

Hunk #4
-------
Why is '[bname,".shx"]' better than '[ bname ".shx" ]' ?

Hunk #6 has already been applied.
-------

Hunk #7
-------
Tests should not rely on other OF packages.
Testing for .dbf and attributes should fail gracefully but OF packages should not be loaded/unloaded behind a user's back.

The other hunks are fine, thanks, I'll apply them soon.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 27 Aug 2016 09:00:52 PM UTC, comment #2: 

Martin,
I see you catched a few other bugs here, thanks.

As to the BoundingBox: the shape files I write are swallowed happily by ArcGIS (my reference). But the shapewrite.m I use (at work) may have other fixes/mods as well. I'll have a look later on.
Syncing that was on my list anyway.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 27 Aug 2016 07:44:48 AM UTC, comment #1: 

Thanks for reporting, Martin.

From your bug descriptions I think the second bug has been fixed some time ago, see the log in [4]. The first one sounds familiar to me, I think it is also covered already [2] in that same commit.

Get shapewrite.m from the repo [3], swap it with the one in your installed package[1], and report back please as to the Bounding Box issue.


Then, two hints:

1. Bugs like this and esp. patches for bugs should rather be reported on the bug tracker, https://savannah.gnu.org/bugs/?func=additem&group=octave
Leave the patch tracker for new functionality and new functions not yet in Octave or an OF package.

2. Before submitting patches, please look first in the on-line repo (for mapping: [4]).

Especially OF packages have slow release dynamics so fixes may take some time before "seeing daylight" in a new package release.
FYI, mapping-1.2.2 is due but JuanPi (geometry package maintainer)and I are busy integrating boolean polygon operations in those packages.


[1] type "which shapewrite" in an Octave terminal to find out where it lives

[2] http://hg.code.sf.net/p/octave/mapping/rev/f68fb8dc1adc

[3] http://hg.code.sf.net/p/octave/mapping/file/b2c06cb10ee4/inst/shapewrite.m

[4] http://hg.code.sf.net/p/octave/mapping
(Easily adapted to other OF packages :-) )

Philip Nienhuis <philipnienhuis>
Group Member
Sat 27 Aug 2016 06:15:13 AM UTC, original submission:  

Hi Philip,

I have recently stumbled upon some bugs in shapewrite and would like to share my fixes. I attach a diff against the version distributed within mapping 1.2.1.
The first bug was with file handling: When a full path is supplied, fileparts returns the directory without trailing separator. During the creation of bname by concatenation this lead to a wrong path and filename.
The second one was with point data that had no bounding box defined. There was a section in the code that seemed to be supposed to handle this situation (creation of a dummy bounding box), but it did not work. My proposed fix creates a bounding box from the highest/lowest coordinate of all points. Is that a problem, i.e. are there cases where a bounding box that extents beyond the boxes would be useful?

Furthermore I have added two tests. They succeed, but the last assert demonstrates that a some space padding happens to string attributes the reason of which I do not no.

I should also add that I have little knowledge about shapefiles. O:-)

Cheers,
Martin

Martin Kunz <mkunz>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #38371:  shapereadDiff.txt added by mkunz (3KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mkunz (Submitted the item)
  • -email is unavailable- added by mkunz
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-10-07 philipnienhuis StatusNeed Info Done
        Open/ClosedOpen Closed
    2016-08-27 philipnienhuis StatusNone Need Info
    2016-08-27 mkunz Attached File- Added shapereadDiff.txt, #38371
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code