Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document the reason for first field always being a DeletionFlag #241

Closed
bayersglassey-zesty opened this issue Apr 22, 2022 · 2 comments
Closed

Comments

@bayersglassey-zesty
Copy link

bayersglassey-zesty commented Apr 22, 2022

Hello! I'm trying to port some code from using the GDAL library to pyshp, because it only needs to read shapefiles, and many things about GDAL are not very nice to deal with.

So I'm liking pyshp so far, but I'm confused why it explicitly prepends a DeletionFlag field to the start of its fields:

self.fields.insert(0, ('DeletionFlag', 'C', 1, 0))

...there are various comments in the source which allude to this, but don't explain why it is done, e.g.

  • "by default, read all fields except the deletion flag, hence "[1:]""
  • "Note that this always includes the DeletionFlag at index 0, regardless of the 'fields' arg."
  • "deletion flag field is always unpacked as first value (see __recordFmt)"
  • "# drop deletion flag from values"

It's a bit confusing when porting from GDAL, because I can take the list of fields parsed from the same shapefile by GDAL and pyshp, and the following is true:

assert gdal_fields == pyshp_fields[1:]

It even turns up in the README, in some code samples and a bugfix ("Fix Reader geo interface including DeletionFlag field in feature properties"):
https://github.com/GeospatialPython/pyshp/blob/d9f917a59a922c6981302f99fad8d94013bcf5cd/README.md

>>> r = shapefile.Reader('shapefiles/test/polygon')

>>> w = shapefile.Writer('shapefiles/test/copy')
>>> w.fields = r.fields[1:] # skip first deletion field

It seems like a sort of minor internal hack which is easy to work around, except that fields is exposed to the user, so the user has to deal with it too.
Can it be documented somewhere? :)

@karimbahgat
Copy link
Collaborator

You're absolutely right. The DeletionFlag is a bit of a historical relic; I believe it had a use in an older Editor class but no longer serves any purpose, so is currently just kept to avoid breaking existing code. Would definitely be better to not expose it to the user, and I'm considering dropping it for a new major release, but since skipping the first field is such a commonly used pattern in the public API it needs to be thought through a little. Either that or more officially supporting the deletion flag in some way. I hadn't realized it wasn't clearly described in the README, so for the time being I've added a paragraph on this, which I think should help. Thanks for the good suggestion!

@bayersglassey-zesty
Copy link
Author

The new README section is perfect, thanks for that! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants