tkrajina / gpxgo

GPX library for golang
Apache License 2.0
94 stars 23 forks source link

Add support for GPX extensions #3

Closed qrtp closed 5 years ago

qrtp commented 6 years ago

Hi @tkrajina, would you be willing to accept this PR from my fork? I'm using it in a project to analyze heart rate zones across Strava workouts, found here:

https://github.com/qrtp/gpx-hr

Would prefer not to reference my fork and pull this back into your branch if you like.

tkrajina commented 6 years ago

@qrtp thanks for your PR. I know it solves a real world problem, but this is also part of a bigger problem on how to handle gpx extensions.

You can visit https://github.com/tkrajina/gpxpy/pull/102 to get a better idea on what exactly the problem is. https://github.com/tkrajina/gpxpy/pull/102 is not for gpxgo, but for gpxpy, but gpxgo is basically a rewrite of gpxpy.

I'll quote @nawagers here from his comment. The goal of gpxpy should be:

I think gpxgo should have the same goals.

Now, your pull request solves only the problem of reading, and only for one specific extensions. That's not to say that I don't want to support heart rates, but I think it should be a part of a generic extensions support in the library.

I'll leave this PR open (but changed slightly the title) just to leave it as a place for discussion about extensions in gpxgo. For gpxpy we decided to leave extensions as bare dom elements, and then eventually programmers can create wrappers around them. Maybe we can use a similar approach in Golang (although the Go stdlib don't have the equivalent of Python's minidom or etree) or we can think of something different here.

Ideas are welcome.