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

Add closed flag to line features #602

Merged
merged 2 commits into from
Aug 5, 2016
Merged

Add closed flag to line features #602

merged 2 commits into from
Aug 5, 2016

Conversation

manthey
Copy link
Contributor

@manthey manthey commented Jul 21, 2016

The closed flag will automatically join the first and last points. The last point in the line need not be duplicated.

Polygons now always use closed lines.

The picking example can take an optional parameter ?closed=true to show closed lines, allowing picking tests on the close segment.

BREAKING CHANGE: Fixed inconsistency in pointSearch. The pointSearch function returned different keys depending on whether data was present or not in both the point and line features. Specifically, when there is no data in a feature, or other pointSearch functions (in quad, polygon, and the base class feature), pointSearch returns {index: [...], found: [...]}. When data is present, the point and line features returned {index: [...], data: [...]}. This has been changed to always return {index: [...], found: [...]}.

Added unit tests for line features.

Resolves issue #599.

The last point in the line need not be duplicated.  The closed flag will automatically join the first and last points.

Polygons now always use closed lines.

The picking example can take an optional parameter ?closed=true to show closed lines, allowing picking tests on the close segment.
BREAKING CHANGE: The pointSearch function returned different keys depending on whether data was present or not in both the pointFeature and the lineFeature.  Specifically, when there is no data in a feature, or other pointSearch functions (in quad, polygon, and the base class feature), pointSearch returns {index: [...], found: [...]}.  When data is present, the point and line features returned {index: [...], data: [...]}.  This has been changed to always return {index: [...], found: [...]}.

Added unit tests for line features.
@codecov-io
Copy link

codecov-io commented Jul 21, 2016

Current coverage is 81.54% (diff: 100%)

Merging #602 into master will increase coverage by 0.82%

@@             master       #602   diff @@
==========================================
  Files            82         82          
  Lines          7419       7436    +17   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           5989       6064    +75   
+ Misses         1430       1372    -58   
  Partials          0          0          

Powered by Codecov. Last update daf9c26...dfc9aae

firstpos = pos;
}
}
if (lineItem.length > 2 && closedFunc(data[i], i)) {
Copy link
Member

Choose a reason for hiding this comment

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

Why passing two arguments to closedFunc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our general method of getting style information from functions is to pass two parameters: the current data item and its index.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For sub-components, like points within a line, the style information is obtained by calling a function with four parameters: the current sub-item (a vertex, for instance), the sub-item's index, the current item (the line), and the item index.

Copy link
Member

Choose a reason for hiding this comment

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

Our general method of getting style information from functions is to pass two parameters: the current data item and its index.

Sure, makes sense.

@aashish24
Copy link
Member

Looking pretty good to me, just had one questions. Also, on returning data, was the data object pointed to the original data object?

@manthey
Copy link
Contributor Author

manthey commented Jul 27, 2016

No. When we called pointSearch, the data entry was an array, each element of which is one of the original values from the original data object. The only change there was renaming pointSearch's resulting value data to found to be consistent. found still is an array with each element one of the original values from the original data object.

@aashish24
Copy link
Member

aashish24 commented Jul 27, 2016

No. When we called pointSearch, the data entry was an array, each element of which is one of the original values from the original data object. The only change there was renaming pointSearch's resulting value data to found to be consistent. found still is an array with each element one of the original values from the original data object.

ah, yes, I see that now, the change is just a key change, the value remains the same.

-      data: found,
+      found: found,

👍

@aashish24
Copy link
Member

Lgtm +1

@manthey manthey merged commit ccef540 into master Aug 5, 2016
@manthey manthey deleted the close-lines branch August 5, 2016 15:50
@manthey manthey mentioned this pull request Aug 5, 2016
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.

3 participants