Tidying (#38)

seanhandley · web-flow · commit 530096908e29 · 2018-12-21T11:47:09.000Z
* Remove unnecessary files.

* Move dependencies to gemspec.

* Add MIT license.

* Move these specs to indexing_spec.rb

* Move bindings base.

* hex ring needs to handle pentagons.

* Functionality improvements.

* Update gem version to match H3.

* Update readme.
diff --git a/.ruby-version b/.ruby-version
diff --git a/Gemfile b/Gemfile
@@ -1,8 +1,2 @@
 source "https://rubygems.org"
-
-ruby "2.4.4" # Ideally, the same one as stuart-api.
-
-gem "rake"
-gem "rspec",        "~> 3.8.0"
-gem "ffi",          "~> 1.9.6"
-gem "rgeo-geojson", "~> 2.1.1"
+gemspec
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -1,10 +1,17 @@
+PATH
+  remote: .
+  specs:
+    h3 (3.2.0)
+      ffi (~> 1.9)
+      rgeo-geojson (~> 2.1)
+
 GEM
   remote: https://rubygems.org/
   specs:
     diff-lcs (1.3)
     ffi (1.9.25)
     rake (12.3.2)
-    rgeo (1.0.0)
+    rgeo (2.0.0)
     rgeo-geojson (2.1.1)
       rgeo (>= 1.0.0)
     rspec (3.8.0)
@@ -20,18 +27,16 @@ GEM
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.8.0)
     rspec-support (3.8.0)
+    yard (0.9.16)
 
 PLATFORMS
   ruby
 
 DEPENDENCIES
-  ffi (~> 1.9.6)
-  rake
-  rgeo-geojson (~> 2.1.1)
-  rspec (~> 3.8.0)
-
-RUBY VERSION
-   ruby 2.4.4p296
+  h3!
+  rake (~> 12.3)
+  rspec (~> 3.8)
+  yard (~> 0.9)
 
 BUNDLED WITH
-   1.17.1
+   1.17.2
diff --git a/LICENSE.md b/LICENSE.md
@@ -0,0 +1,21 @@
+MIT License
+
+Copyright (c) 2018 Stuart Delivery
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
diff --git a/README.md b/README.md
@@ -1,10 +1,12 @@
 # H3 Ruby
 
-[![Build Status](https://travis-ci.org/StuartApp/h3_ruby.svg?branch=master)](https://travis-ci.org/seanhandley/h3_ruby)
+![h3](https://user-images.githubusercontent.com/98526/50283275-48177300-044d-11e9-8337-eba8d3cc88a2.png)
+
+[![Build Status](https://travis-ci.org/StuartApp/h3_ruby.svg?branch=master)](https://travis-ci.org/seanhandley/h3_ruby) [![Maintainability](https://api.codeclimate.com/v1/badges/c55e1f67421eba8af8d0/maintainability)](https://codeclimate.com/repos/5c18b7f49bc79a02a4000d81/maintainability)
 
 Ruby bindings for Uber's [H3 library](https://uber.github.io/h3/).
 
-TODO: Add more wrapper functions.
+Please consult the H3 documentation for a full explanation of terminology and concepts.
 
 ## Getting Started
 
@@ -14,24 +16,26 @@ Install the build dependencies as instructed here: https://github.com/uber/h3#in
 
 Do *not* follow the Compilation Steps. Instead, use the following:
 
-    git clone git@github.com:uber/h3.git
-    cd h3
+    git clone git@github.com:uber/h3.git h3_build
+    cd h3_build
     cmake . -DBUILD_SHARED_LIBS=true
     make
     sudo make install
 
+The key difference is the `BUILD_SHARED_LIBS` option.
+
 ## Installing
 
     gem install h3
 
 or
 
     # Gemfile
-    gem "h3", "~> 0.0.1pre"
+    gem "h3", "~> 3.2"
 
-## Running Specs
+## Documentation
 
-    bin/rspec spec
+https://www.rubydoc.info/github/StuartApp/h3_ruby
 
 ## Usage
 
@@ -41,6 +45,7 @@ H3.geo_to_h3([53.959130, -1.079230], 8).to_s(16)
 # => "8819429a9dfffff"
 ```
 
-## GeoJSON
+## Running Specs
+
+    rake
 
-Note that methods returning GeoJSON contain coordinates in the form `[lon, lat]`, whereas H3 coordinates use `[lat, lon]`.
diff --git a/bin/console b/bin/console
diff --git a/bin/rspec b/bin/rspec
diff --git a/bin/run b/bin/run
diff --git a/h3.gemspec b/h3.gemspec
@@ -1,21 +1,21 @@
-$: << File.expand_path("lib", __dir__)
-require "h3/version"
+require_relative "lib/h3/version"
 
 Gem::Specification.new do |spec|
   spec.name     = "h3"
   spec.version  = H3::VERSION
-  spec.licenses = ["Nonstandard"] # Avoids a warning when building the gem.
+  spec.licenses = ["MIT"]
   spec.summary  = "C Bindings for Uber's H3 library"
   spec.homepage = "https://github.com/StuartApp/h3_ruby"
   spec.authors  = ["Lachlan Laycock", "Sean Handley"]
   spec.email    = "l.laycock@stuart.com"
 
-  spec.extensions = %w(ext/h3/extconf.rb)
-  spec.require_paths = %w(lib)
-  spec.files = %w(
-    ext/h3/extconf.rb
-    ext/h3/h3.c
-    lib/h3.rb
-    lib/h3/version.rb
-  )
+  spec.required_ruby_version = "> 2.3"
+  spec.files = `git ls-files`.split("\n")
+
+  spec.add_runtime_dependency "ffi", "~> 1.9"
+  spec.add_runtime_dependency "rgeo-geojson", "~> 2.1"
+
+  spec.add_development_dependency "rake", "~> 12.3"
+  spec.add_development_dependency "rspec", "~> 3.8"
+  spec.add_development_dependency "yard", "~> 0.9"
 end
diff --git a/lib/h3.rb b/lib/h3.rb
@@ -1,9 +1,7 @@
 require "ffi"
 require "rgeo/geo_json"
 
-require "h3/binding_base"
-require "h3/bindings/structs"
-require "h3/bindings/private"
+require "h3/bindings"
 require "h3/geo_json"
 require "h3/hierarchy"
 require "h3/indexing"
diff --git a/lib/h3/binding_base.rb b/lib/h3/binding_base.rb
diff --git a/lib/h3/bindings.rb b/lib/h3/bindings.rb
@@ -0,0 +1,12 @@
+require "h3/bindings/base"
+require "h3/bindings/structs"
+require "h3/bindings/private"
+
+module H3
+  # Internal bindings related modules and classes.
+  #
+  # These are intended to be used by the library's public methods
+  # and not to be used directly by client code.
+  module Bindings
+  end
+end
diff --git a/lib/h3/bindings/base.rb b/lib/h3/bindings/base.rb
@@ -0,0 +1,15 @@
+module H3
+  module Bindings
+    # Base for FFI Bindings.
+    #
+    # When extended, this module sets up FFI to use the H3 C library.
+    module Base
+      def self.extended(base)
+        base.extend FFI::Library
+        base.ffi_lib ["libh3", "libh3.1"]
+        base.typedef :ulong_long, :h3_index
+        base.const_set('H3_INDEX', :ulong_long)
+      end
+    end
+  end
+end
diff --git a/lib/h3/bindings/private.rb b/lib/h3/bindings/private.rb
@@ -5,7 +5,7 @@ module Bindings
     # This module provides bindings that do not have to be invoked directly by clients
     # of the library. They are used only internally to provide related public interface.
     module Private
-      extend H3::BindingBase
+      extend H3::Bindings::Base
 
       attach_function :compact, [:pointer, :pointer, :int], :bool
       attach_function :geo_to_h3, :geoToH3, [ Bindings::Structs::GeoCoord.by_ref, :int ], :h3_index
@@ -20,7 +20,7 @@ module Private
       attach_function :hex_range, :hexRange, [ :h3_index, :int, :pointer ], :bool
       attach_function :hex_range_distances, :hexRangeDistances, [:h3_index, :int, :pointer, :pointer], :bool
       attach_function :hex_ranges, :hexRanges, [ :pointer, :int, :int, :pointer ], :bool
-      attach_function :hex_ring, :hexRing, [:h3_index, :int, :pointer], :void
+      attach_function :hex_ring, :hexRing, [:h3_index, :int, :pointer], :bool
       attach_function :k_ring, :kRing, [:h3_index, :int, :pointer], :void
       attach_function :k_ring_distances, :kRingDistances, [:h3_index, :int, :pointer, :pointer], :bool
       attach_function :max_polyfill_size, :maxPolyfillSize, [Bindings::Structs::GeoPolygon.by_ref, :int], :int
diff --git a/lib/h3/geo_json.rb b/lib/h3/geo_json.rb
@@ -84,9 +84,11 @@ def geo_json_to_coordinates(input)
                     elsif geom.respond_to?(:coordinates) # polygon
                       geom.coordinates
                     else
-                      raise "Could not parse given input. Please use a GeoJSON polygon."
+                      failed_to_parse!
                     end
-      swap_lat_lon(coordinates)
+      swap_lat_lon(coordinates) || failed_to_parse!
+    rescue JSON::ParserError
+      failed_to_parse!
     end
 
     # Convert a nested array of coordinates to a GeoJSON document
@@ -139,12 +141,14 @@ def coordinates_to_geo_json(coordinates)
       coordinates = swap_lat_lon(coordinates)
       outer_coords, *inner_coords = coordinates
       factory = RGeo::Cartesian.simple_factory
-      exterior = factory.linear_ring(outer_coords.map! { |lon, lat| factory.point(lon, lat) })
+      exterior = factory.linear_ring(outer_coords.map { |lon, lat| factory.point(lon, lat) })
       interior_rings = inner_coords.map do |polygon|
         factory.linear_ring(polygon.map { |lon, lat| factory.point(lon, lat) })
       end
       polygon = factory.polygon(exterior, interior_rings)
       RGeo::GeoJSON.encode(polygon).to_json
+    rescue RGeo::Error::InvalidGeometry, NoMethodError
+      invalid_coordinates!
     end
 
     private
@@ -153,5 +157,13 @@ def coordinates_to_geo_json(coordinates)
     def swap_lat_lon(coordinates)
       coordinates.map { |polygon| polygon.map { |x, y| [y, x] } }
     end
+
+    def failed_to_parse!
+      raise ArgumentError, "Could not parse given input. Please use a GeoJSON polygon."
+    end
+
+    def invalid_coordinates!
+      raise ArgumentError, "Could not parse given coordinates."
+    end
   end
 end
diff --git a/lib/h3/hierarchy.rb b/lib/h3/hierarchy.rb
@@ -3,7 +3,7 @@ module H3
   #
   # @see https://uber.github.io/h3/#/documentation/api-reference/hierarchy
   module Hierarchy
-    extend H3::BindingBase
+    extend H3::Bindings::Base
 
     # @!method h3_to_parent(h3_index, parent_resolution)
     #
diff --git a/lib/h3/indexing.rb b/lib/h3/indexing.rb
@@ -20,11 +20,14 @@ module Indexing
     #
     # @return [Integer] H3 index.
     def geo_to_h3(coords, resolution)
-      raise TypeError unless coords.is_a?(Array)
-      raise ArgumentError unless coords.count == 2
-      raise RangeError if coords.any? { |d| d > 1000000 }
+      raise ArgumentError unless coords.is_a?(Array) && coords.count == 2
 
       lat, lon = coords
+
+      if lat > 90 || lat < -90 || lon > 180 || lon < -180
+        raise(ArgumentError, "Invalid coordinates")
+      end
+
       coords = Bindings::Structs::GeoCoord.new
       coords[:lat] = degs_to_rads(lat)
       coords[:lon] = degs_to_rads(lon)
diff --git a/lib/h3/inspection.rb b/lib/h3/inspection.rb
@@ -3,9 +3,10 @@ module H3
   #
   # @see https://uber.github.io/h3/#/documentation/api-reference/inspection
   module Inspection
-    extend H3::BindingBase
+    extend H3::Bindings::Base
 
     H3_TO_STR_BUF_SIZE = 32
+    private_constant :H3_TO_STR_BUF_SIZE
 
     # @!method h3_resolution(h3_index)
     #
diff --git a/lib/h3/miscellaneous.rb b/lib/h3/miscellaneous.rb
@@ -3,7 +3,7 @@ module H3
   #
   # @see https://uber.github.io/h3/#/documentation/api-reference/miscellaneous
   module Miscellaneous
-    extend H3::BindingBase
+    extend H3::Bindings::Base
 
     # @!method degs_to_rads(degs)
     #
diff --git a/lib/h3/regions.rb b/lib/h3/regions.rb
@@ -3,8 +3,7 @@ module H3
   #
   # @see https://uber.github.io/h3/#/documentation/api-reference/regions 
   module Regions
-    extend H3::BindingBase
-
+    extend H3::Bindings::Base
 
     # Derive the maximum number of H3 indexes that could be returned from the input.
     #
@@ -136,6 +135,7 @@ def polyfill(geo_polygon, resolution)
     #
     # @return [Array<Array<Array<Float>>>] Nested array of coordinates.
     def h3_set_to_linked_geo(h3_indexes)
+      h3_indexes.uniq!
       linked_geo_polygon = Bindings::Structs::LinkedGeoPolygon.new
       FFI::MemoryPointer.new(H3_INDEX, h3_indexes.size) do |hexagons_ptr|
         hexagons_ptr.write_array_of_ulong_long(h3_indexes)
diff --git a/lib/h3/traversal.rb b/lib/h3/traversal.rb
@@ -3,7 +3,7 @@ module H3
   #
   # @see https://uber.github.io/h3/#/documentation/api-reference/traversal
   module Traversal
-    extend H3::BindingBase
+    extend H3::Bindings::Base
 
     # @!method max_kring_size(k)
     #
diff --git a/lib/h3/unidirectional_edges.rb b/lib/h3/unidirectional_edges.rb
@@ -3,7 +3,7 @@ module H3
   # 
   # @see https://uber.github.io/h3/#/documentation/api-reference/unidirectional-edges
   module UnidirectionalEdges
-    extend H3::BindingBase
+    extend H3::Bindings::Base
 
     # @!method h3_indexes_neighbors?(origin, destination)
     #
diff --git a/lib/h3/version.rb b/lib/h3/version.rb
@@ -1,3 +1,3 @@
 module H3
-  VERSION = "0.0.1pre"
+  VERSION = "3.2.0"
 end
diff --git a/spec/geo_json_spec.rb b/spec/geo_json_spec.rb
diff --git a/spec/indexing_spec.rb b/spec/indexing_spec.rb
diff --git a/spec/miscellaneous_spec.rb b/spec/miscellaneous_spec.rb
diff --git a/spec/traversal_spec.rb b/spec/traversal_spec.rb
