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

Allow customisation by transformers #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JarnoLeConte
Copy link
Contributor

@JarnoLeConte JarnoLeConte commented May 18, 2016

Code is now more generalised and has explicit build functions for the elements:
<wpt>, <trkpt>, <rtept>, <trk>, <rte>, <gpx>

Additionally, developers could define options.transform to attach transform functions to each of these build functions. These transformers will post process the data and give the flexibility to set all fields defined in the GPX v1.1 specification.

This way developers could easily define their own helpers to fit their needs. The functions get_feature_title(), get_feature_description() and get_feature_coord_times() could have been implemented this way. Moreover this solves my initial problem where properties.coordTimes was defined on my features instead of properties.times.

EXAMPLES

  • get_feature_title() implemented by a transformer to set the name of a waypoint <wpt>.
function set_title(wpt, feature, coord, index) {
  wpt.name = feature.properties.name;
}

togpx(geojson, {
  transform: {
    wpt: set_title
  }
})
  • times + elevation implemented by a transformer on each track point <trkpt>.
function set_data(trkpt, feature, coord, index) {
  trkpt.time = feature.properties.coordTimes[index];
  trkpt.ele = coord[2];
}

togpx(geojson, {
  transform: {
    trkpt: set_data
  }
})
function set_extensions(rte, feature, coords) {
  rte.extensions = [{
    distance: feature.properties.distance,
    viaPoints: coords.length
  }];
}

togpx(geojson, {
  transform: {
    rte: set_extensions
  }
})
  • Transform final JXON object
functions modify(gpx, features) {
  gpx.wpt = [ ... ] // edit waypoints
  gpx.trk = [ ... ] // edit tracks
  gpx.rte = [ ... ] // edit routes
  gpx.metadata = [ ... ] // edit meta data
  gpx["@xmlns:trp"] = "http://www.garmin.com/xmlschemas/TripExtensions/v1"; // add attributes
}

togpx(geojson, {
  transform: {
    gpx: modify
  }
})

API

available transformers

togpx(geojson, {
  transform: {
    wpt: function(wpt, feature, coord, index) { // way point (Point)
      ...
    },
    trkpt: function(trkpt, feature, coord, index) { // track point (Point)
      ...
    },
    rtept: function(rtept, feature, coord, index) { // route point (Point)
      ...
    },
    trk: function(trk, feature, coordsList) { // track (MultiLineString)
      ...
    },
    rte: function(rte, feature, coords) { // route (LineString)
      ...
    },
    gpx: function(gpx, features) { 
      ...
    },
  }
})

@JarnoLeConte JarnoLeConte mentioned this pull request May 25, 2016
});
break;
default:
console.log("warning: unsupported geometry type: "+f.geometry.type);
}
};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the ; is superfluous here, right? (as it's the end of the mapFeature function definition)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right.

@tyrasd
Copy link
Owner

tyrasd commented May 25, 2016

This looks like great improvements, thank you very much!

But can you please complete a few more things before I can merge it (and make it into a v0.6 release):

  1. rebase the code onto current master and fix the merge conflicts
  2. modify the logic in the defaults helper method, so that it matches ES6's Object.assign (just removing the if condition should do the trick), rename it to assign (or so) and use it also override the default options at the very beginning of the code
  3. document the new functionality in the project's readme, i.e. the new options (gpx.* flags and transforms) and maybe a dedicated chapter about how to use the transforms with one or two examples.

@JarnoLeConte
Copy link
Contributor Author

Yes, I'll do that. Some comments:
2. Personally I would like to use the underscore for these things, like the _.defaults(...) function for example. But maybe you wouldn't depend on libraries?
3. I will start some documentation, but my English isn't very good, so I will appreciate if you would like to fix it afterwards to make it human readable ^^

@tyrasd
Copy link
Owner

tyrasd commented May 25, 2016

For a small library such as this one, I usually try to keep external dependencies to an absolute minimum.
I'll look over the documentation, of course.

@stevenhorner
Copy link

stevenhorner commented Mar 31, 2017

Just wondering if this pull request is going to be merged. I was about to try and add the ability to save routes to your code when I noticed @JarnoLeConte PR which includes this. Must admit I haven't done a PR before either so unsure why it stalled?

nrenner added a commit to nrenner/brouter-web that referenced this pull request Feb 19, 2021
Use togpx fork for now (outdated), see PR:
tyrasd/togpx#11
nrenner added a commit to nrenner/brouter-web that referenced this pull request Feb 26, 2021
Use togpx fork for now (outdated), see PR:
tyrasd/togpx#11
nrenner added a commit to nrenner/brouter-web that referenced this pull request Mar 1, 2021
Use togpx fork for now (outdated), see PR:
tyrasd/togpx#11
nrenner added a commit to nrenner/togpx that referenced this pull request Mar 9, 2021
- Remove superfluous `;` (and another)
- Reverse `default` logic to match Object.assign and options init
- Rename `default` to `assign`
- Replace default options overrride on init
@nrenner
Copy link

nrenner commented Mar 9, 2021

See #16 for a merge of this PR for current master, with improvements applied, but still lacking documentation.

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

Successfully merging this pull request may close these issues.

4 participants