Port Meta Tests and Functions #12

baparham · 2020-12-12T06:31:27Z

lukas-h · 2021-11-02T12:19:51Z

Hi @baparham, any updates?

baparham · 2021-11-08T07:59:45Z

No 😳 it's been a rather busy year, but I can spend some time on this, I've been wanting to get back into flutter/dart development lately so your ping is timely. I'll block out some time this week to devote here.

lukas-h · 2021-11-08T09:50:09Z

Nice! I'm also slowly starting to work on other parts of the library again, e.g. null-safety support which is more than overdue

tobrun · 2021-11-20T06:24:53Z

The most useful meta function to now work on will be coordEach. It's heavily used by other turf functions. Happy to work on that one but might tip my toes in the water with an easy meta function to get started :)

edit: have implementation ready for propEach but depends on merging #13 first

lukas-h · 2021-11-21T20:52:56Z

When implementing the functions, please let's keep in mind that my GeoJSON implementation follows a very strict class representation of the RFC standard. I would prefer to have stricter types for the functions, e.g.:

- void geomEach(dynamic geoJSON, GeomEachCallback callback) {
+ void geomEach(GeoJSONObject geoJSON, GeomEachCallback callback) {

also, see my comments here: #24 and here: #14 (comment)

here you can see all the classes and their inheritance in a tree diagram:

baparham · 2021-11-22T07:25:37Z

I agree @lukas-h that these should be typed stricter. Currently the GeoJSONObject doesn't include features, so perhaps that's the first step, is to bring that abstract class up to snuff?

I can try and implement this in a new PR, I'll paste that in here if I get something that works

baparham · 2021-11-22T07:37:55Z

It seems like I can use some runtime typechecking, and casting with as but I think that means that most of the geomEach function will get rewritten since dart can't handle the way turf.js used the isFeature[Collection] booleans to handle runtime type checking.

e.g. this works:

  bool isFeatureCollection = instanceof(geoJSON, FeatureCollection);
  bool isFeature = instanceof(geoJSON, Feature);
  int stop = 1;
  if (isFeatureCollection) {
    stop = (geoJSON as FeatureCollection).features.length;
  }

but then further down we have things like this that do not work:

  for (var i = 0; i < stop; i++) {
    geometryMaybeCollection = (isFeatureCollection
        ? geoJSON.features[i].geometry
        : (isFeature ? geoJSON.geometry : geoJSON));
    featureProperties = (isFeatureCollection
        ? geoJSON.features[i].properties
        : (isFeature ? geoJSON.properties : {}));
    featureBBox = (isFeatureCollection
        ? geoJSON.features[i].bbox
        : (isFeature ? geoJSON.bbox : null));
    featureId = (isFeatureCollection
        ? geoJSON.features[i].id
        : (isFeature ? geoJSON.id : null));
    isGeometryCollection = (geometryMaybeCollection != null)
        ? geometryMaybeCollection.type == GeoJSONObjectTypes.geometryCollection
        : false;
...

since features is only available on the subclass FeatureCollection and not on the abstract GeoJSONObject class :-(

I think I'll just go ahead and refactor this code to meet our intended purpose, be more readable, yet diverge from simple 1:1 implementation mapping from turf.js

lukas-h · 2021-11-22T07:40:23Z

@baparham That's a little bit more difficult than that..

Not all subtypes of GeoJSONObject should have the same attributes following the RFC standard.
For example Point, Polygon, etc. have coordinates, GeometryCollection has geometries, Feature has geometry, FeatureCollection has features.

Maybe we need to work more with type casting as or checking is for subclasses of GeoJSONObject..

baparham · 2021-11-22T07:45:39Z

I think it works to simply trust the type field and then cast as needed using the as operator. I tried to make a simple proof of concept, but my gh credentials locally are blocking me from pushing 🙄 so give me a couple more minutes, haha

baparham · 2021-11-22T07:47:35Z

what about this method? #29

It's soooo ugly and needs to be refactored to use more focused helper functions I think, but to test the concept, I think it works.

baparham · 2021-11-22T07:58:06Z

here you can see all the classes and their inheritance in a tree diagram:

Regarding naming, does GeometryObject and Geometry work as replacements for Geometry and GeometryType (respectively) perhaps to try and more closely parallel the GeoJSONObject?

[edit:] I mean, I am not really a fan of redundant naming conventions like *Object and *Type but we have to differentiate these higher order types somehow

lukas-h · 2021-11-22T08:20:22Z

I see why the naming can be a little bit confusing. We should definitely think of a naming convention.

I think GeoJSONObject can stay as it is.
Geometry to GeometryObject seems okay!
Changing GeometryType to just Geometry is confusing, I think.
We should find a new name for CoordinateType as well.

Background: the RFC standard defines different groups of GeoJSON object types:

GeoJSON Objects, currently covered by GeoJSONObject, basically all usable GeoJSON types: [Point, Multipoint, Polygon, MultiPolygon, LineString, MultiLineString, GeometryCollection, Feature, FeatureCollection]
Geometry Objects, currently covered by Geometry, a subset of the types that actually include some kind of geometry(/-ies): [Point, Multipoint, Polygon, MultiPolygon, LineString, MultiLineString, GeometryCollection]
Additionally I created another abstract type for all classes that have a coordinates attributes, which I called GeometryType (which extends Geometry): [Point, Multipoint, Polygon, MultiPolygon, LineString, MultiLineString]

tobrun · 2021-12-13T08:12:45Z

Finally getting back to my original attempt of implementing coordEach to unblock the other APIs. I have it most of it ported from JS implementation but I'm noticing a difference in setup between Dart vs JS setup of geomEach. Am I correct to assume, we rather want the _ShortCircuit approach as with geomEach? and I should drop the JS specific optimizations as commented in This logic may look a little weird?

lukas-h · 2021-12-14T10:44:20Z

@tobrun Maybe let's first implement it like the geomEach.
But later on we could test the performance of different implementation in a benchmark suite, as proposed in #32

baparham · 2021-12-14T20:18:56Z

I'll get started on flattenEach for now

lukas-h · 2021-12-15T01:00:52Z

I have a proposal:

To have all the meta functions like as independent / standalone functions, seems to me very "non-OOP", not "Dart-like", but instead like a more common style for JavaScript.

My idea is, to glue the meta functions (at for least those, that actually make sense) to the GeoJSON classes directly.

For the user, the API could look like this:

var featColl = FeatureCollection.fromJson(json);

featColl.geomEach(
// ...
);

I started to implement this for the geomEach function in two different ways:

directly inside each of the classes inheriting from GeoJSONObject in geojson.dart:

turf_dart/lib/src/geojson.dart

Line 68 in b02af15

void geomEachImpl1(GeomEachCallback callback);
as extension methods on all of the classes in this file:

turf_dart/lib/src/geomeach_extension.dart

Line 6 in b02af15

void geomEachImpl2(GeomEachCallback callback) {

You can see the changes in this commit: b02af15

It's not yet debugged, and there could still be some hurdles with the implementation,
but it already illustrates my idea very well.

Please let me know what you think!

@baparham @tobrun

tobrun · 2021-12-15T09:03:29Z

To have all the meta functions like as independent / standalone functions, seems to me very "non-OOP", not "Dart-like", but instead like a more common style for JavaScript.

I like the idea, the only things I could counter to this proposal are:

we would be adding many APIs to GeoJSON classes that might make it more complicated for developers to navigate (eg. code completion). GeoJSON, for non-geospatial developers, is already hard to initially grasp conceptually.
the implementation wouldn't be a direct match to the GeoJSON spec anymore

I don't think these reasons justify not doing it but I did want to provide that angle as input.

as extension methods on all of the classes in this file

I would vote for an extension approach. In the past I had libraries that were dependent on the java geojson implementation but weren't allowed to pull in turf-java due to binary size constraints. Having them exposed as extension functions will give us the flexibility to pull out geojson into an independent library and have a separate extension function library to provide turf functionality. Are there any downsides to use extension functions?

baparham · 2021-12-15T20:36:17Z

I agree with @tobrun that we don't want to arbitrarily pollute the namespace with stuff people may not care about, and your extensions approach seems to address this concern directly, that if one wants to use the syntactic sugar, they can import the extensions.

I am not very familiar with extensions as a concept, but I do not immediately see downsides. Perhaps if someone is copying example code that takes advantage of the extensions, they may not do the extra import needed, and I don't know if their IDE would suggest such an import.

lukas-h · 2021-12-15T22:41:03Z

I totally agree with both of you! Let’s keep the GeoJSON object representation in geojson.dart slim!

But there’s one catch with the extension method approach:
We can’t add method overrides of a function declaration (e.g. void geomEach…);) in GeoJSONObject to each of the extensions. We can only put concrete method implementations into the extensions.
This causes some weird type casting: as pointed out by @baparham here:
b02af15#r61986415

lukas-h · 2021-12-15T22:45:06Z

Are there any downsides to use extension functions?

what I described in my other comment
Possibly this:

Perhaps if someone is copying example code that takes advantage of the extensions, they may not do the extra import needed, and I don't know if their IDE would suggest such an import.

baparham · 2021-12-16T07:37:11Z

Is there a way to use the actual implementation via the extension method?

import 'package:turf/src/meta.dart';
import 'package:turf/helpers.dart';

extension GeomEachGeoJSONObject on GeoJSONObject {
  void geomEachImpl2(GeomEachCallback callback) {
    geomEach(this, callback);
  }
}

baparham · 2021-12-16T07:37:27Z

I don't like that having the implementation in two places

lukas-h · 2021-12-16T16:25:18Z

I don't like that having the implementation in two places

Totally agree. Tried to incorporate that in this commit: 3243325

Is there a way to use the actual implementation via the extension method?

import 'package:turf/src/meta.dart';
import 'package:turf/helpers.dart';

extension GeomEachGeoJSONObject on GeoJSONObject {
  void geomEachImpl2(GeomEachCallback callback) {
    geomEach(this, callback);
  }
}

I'm not sure, if your (@baparham) idea here using the geomEach top level function or my if implementation in 3243325 is better.

my implementation skips the type checking in the top level function geomEach
your implementation is much cleaner and does not expose all of geomEach's implementation in meta.

lukas-h · 2021-12-19T16:55:00Z

getCoord

getCoords

getGeom

getType

what about the other functions inside turf invariant? I guess @baparham you took the tasks from the README.
I think, we should update the README, because since then, new modules have been created

In my opinion, most of the functions in turf invariant are not even needed in Dart, because of the better type system.
For example getGeom, which just takes a Feature as a param and returns its geometry.

baparham · 2021-12-19T17:52:48Z

Yeah those sound like type guards that aren't needed in a typed language, I agree.

…

On Sun, Dec 19, 2021, 5:55 PM Lukas Himsel ***@***.***> wrote: - getCoord - getCoords - getGeom - getType what about the other functions inside turf invariant? https://github.com/Turfjs/turf/tree/master/packages/turf-invariant In my opinion, some of them are not even needed in Dart, because of the better type system. For example getGeom which just takes a Feature as a param and returns its geometry. — Reply to this email directly, view it on GitHub <#12 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFFP3P3PRRFR2QTFWUHHC3URYE65ANCNFSM4UXYWOUQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

lukas-h · 2022-04-01T16:27:33Z

Will close this, because we are done with this milestone 🎉

baparham self-assigned this Dec 12, 2020

baparham mentioned this issue Jan 1, 2021

Add geomEach meta function #13

Merged

lukas-h mentioned this issue Nov 21, 2021

[meta] add implementation and test for featureEach and propEach #24

Merged

tobrun mentioned this issue Nov 22, 2021

featureEach, remove dynamic, use GeoJSONObject instead #27

Closed

lukas-h mentioned this issue Dec 19, 2021

Meta function extension methods #52

Closed

16 tasks

lukas-h pinned this issue Feb 25, 2022

lukas-h assigned lukas-h and armantorkzaban Feb 28, 2022

lukas-h mentioned this issue Mar 2, 2022

Split up lib/src/meta.dart into different files #60

Closed

lukas-h added this to the Implementation of Meta functions and tests milestone Mar 2, 2022

This was referenced Mar 19, 2022

Port cluster functions and tests #69

Merged

Prepare for release 0.0.5 #66

Closed

lukas-h closed this as completed Apr 1, 2022

lukas-h unpinned this issue Apr 4, 2022

lukas-h mentioned this issue Apr 4, 2022

Meta functions extension methods (attempt 2) #73

Merged

16 tasks

lukas-h mentioned this issue Apr 12, 2022

Update functions and packages in README.md #67

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port Meta Tests and Functions #12

Port Meta Tests and Functions #12

baparham commented Dec 12, 2020 •

edited by armantorkzaban

Loading

lukas-h commented Nov 2, 2021

baparham commented Nov 8, 2021

lukas-h commented Nov 8, 2021

tobrun commented Nov 20, 2021 •

edited

Loading

lukas-h commented Nov 21, 2021 •

edited

Loading

baparham commented Nov 22, 2021

baparham commented Nov 22, 2021

lukas-h commented Nov 22, 2021 •

edited

Loading

baparham commented Nov 22, 2021

baparham commented Nov 22, 2021 •

edited

Loading

baparham commented Nov 22, 2021 •

edited

Loading

lukas-h commented Nov 22, 2021 •

edited

Loading

tobrun commented Dec 13, 2021

lukas-h commented Dec 14, 2021 •

edited

Loading

baparham commented Dec 14, 2021

lukas-h commented Dec 15, 2021 •

edited

Loading

tobrun commented Dec 15, 2021

baparham commented Dec 15, 2021

lukas-h commented Dec 15, 2021 •

edited

Loading

lukas-h commented Dec 15, 2021

baparham commented Dec 16, 2021

baparham commented Dec 16, 2021

lukas-h commented Dec 16, 2021

lukas-h commented Dec 19, 2021 •

edited

Loading

baparham commented Dec 19, 2021 via email

lukas-h commented Apr 1, 2022

Port Meta Tests and Functions #12

Port Meta Tests and Functions #12

Comments

baparham commented Dec 12, 2020 • edited by armantorkzaban Loading

*each

*reduce

other

lukas-h commented Nov 2, 2021

baparham commented Nov 8, 2021

lukas-h commented Nov 8, 2021

tobrun commented Nov 20, 2021 • edited Loading

lukas-h commented Nov 21, 2021 • edited Loading

baparham commented Nov 22, 2021

baparham commented Nov 22, 2021

lukas-h commented Nov 22, 2021 • edited Loading

baparham commented Nov 22, 2021

baparham commented Nov 22, 2021 • edited Loading

baparham commented Nov 22, 2021 • edited Loading

lukas-h commented Nov 22, 2021 • edited Loading

tobrun commented Dec 13, 2021

lukas-h commented Dec 14, 2021 • edited Loading

baparham commented Dec 14, 2021

lukas-h commented Dec 15, 2021 • edited Loading

tobrun commented Dec 15, 2021

baparham commented Dec 15, 2021

lukas-h commented Dec 15, 2021 • edited Loading

lukas-h commented Dec 15, 2021

baparham commented Dec 16, 2021

baparham commented Dec 16, 2021

lukas-h commented Dec 16, 2021

lukas-h commented Dec 19, 2021 • edited Loading

baparham commented Dec 19, 2021 via email

lukas-h commented Apr 1, 2022

baparham commented Dec 12, 2020 •

edited by armantorkzaban

Loading

tobrun commented Nov 20, 2021 •

edited

Loading

lukas-h commented Nov 21, 2021 •

edited

Loading

lukas-h commented Nov 22, 2021 •

edited

Loading

baparham commented Nov 22, 2021 •

edited

Loading

baparham commented Nov 22, 2021 •

edited

Loading

lukas-h commented Nov 22, 2021 •

edited

Loading

lukas-h commented Dec 14, 2021 •

edited

Loading

lukas-h commented Dec 15, 2021 •

edited

Loading

lukas-h commented Dec 15, 2021 •

edited

Loading

lukas-h commented Dec 19, 2021 •

edited

Loading