best_prefix produces invalid results (#347)

Author: olbrich

lib/ruby_units/unit.rb

@@ -1499,12 +1499,22 @@ def coerce(other)
       [self.class.new(other), self]
     end
 
-    # returns a new unit that has been scaled to be more in line with typical usage.
+    # Returns a new unit that has been scaled to be more in line with typical usage. This is highly opinionated and not
+    # based on any standard. It is intended to be used to make the units more human readable.
+    #
+    # Some key points:
+    # * Units containing 'kg' will be returned as is. The prefix in 'kg' makes this an odd case.
+    # * It will use `centi` instead of `milli` when the scalar is between 0.01 and 0.001
+    #
+    # @return [Unit]
     def best_prefix
       return to_base if scalar.zero?
+      return self if units.include?('kg')
 
       best_prefix = if kind == :information
                       self.class.prefix_values.key(2**((::Math.log(base_scalar, 2) / 10.0).floor * 10))
+                    elsif ((1/100r)..(1/10r)).cover?(base_scalar)
+                      self.class.prefix_values.key(1/100r)
                     else
                       self.class.prefix_values.key(10**((::Math.log10(base_scalar) / 3.0).floor * 3))
                     end

spec/ruby_units/unit_spec.rb

@@ -2262,10 +2262,17 @@
     specify { expect(RubyUnits::Unit.new('23 m').div(RubyUnits::Unit.new('2 m'))).to eq(11) }
   end
 
-  context '#best_prefix' do
-    specify { expect(RubyUnits::Unit.new('1024 KiB').best_prefix).to eq(RubyUnits::Unit.new('1 MiB')) }
-    specify { expect(RubyUnits::Unit.new('1000 m').best_prefix).to eq(RubyUnits::Unit.new('1 km')) }
-    specify { expect { RubyUnits::Unit.new('0 m').best_prefix }.to_not raise_error }
+  describe '#best_prefix' do
+    it { expect(RubyUnits::Unit.new('1024 KiB').best_prefix).to have_attributes(scalar: 1, units: 'MiB') }
+    it { expect(RubyUnits::Unit.new('1000 m').best_prefix).to have_attributes(scalar: 1, units: 'km') }
+    it { expect(RubyUnits::Unit.new('1/1000 m').best_prefix).to have_attributes(scalar: 1.0, units: 'mm') }
+    it { expect(RubyUnits::Unit.new('1/100000 m').best_prefix).to have_attributes(scalar: 10, units: 'um') }
+    it { expect(RubyUnits::Unit.new('0 m').best_prefix).to have_attributes(scalar: 0, units: 'm') }
+    it { expect(RubyUnits::Unit.new('1000 kg').best_prefix).to have_attributes(scalar: 1000, units: 'kg') }
+    it { expect(RubyUnits::Unit.new('1_000_000 kg').best_prefix).to have_attributes(scalar: 1_000_000, units: 'kg') }
+    it { expect(RubyUnits::Unit.new('1/100 m').best_prefix).to have_attributes(scalar: 1, units: 'cm') }
+    it { expect(RubyUnits::Unit.new('1/10 m').best_prefix).to have_attributes(scalar: 10, units: 'cm') }
+    it { expect(RubyUnits::Unit.new('0.234 s').best_prefix).to have_attributes(scalar: 234, units: 'ms') }
   end
 
   context 'Time helper functions' do