code cleanup thanks to reek

README.md

@@ -1 +1,56 @@
-ruby-avm-library
+## The Ruby AVM Library
+
+The Astronomy Visualization Metadata (AVM) standard is an extension of the Adobe XMP format. This
+extension adds information to an astronomical image that describes the scientific data and methods
+of collection that went in to producing the image. This Ruby library assists in reading the metadata from
+XMP documents and writing out AVM data as a new XMP file.
+
+## Installing the library
+
+### From Bundler
+
+In your Gemfile:
+
+    gem 'ruby-avm-library'
+
+To use the current development version:
+
+    gem 'ruby-avm-library', :git => ''
+
+### From RubyGems
+
+    gem install ruby-avm-library
+
+## Basic usage
+
+### Reading an XMP file
+
+    require 'avm/image'
+    
+    image = AVM::Image.from_xml(File.read('my-file.xmp'))
+    
+    puts image.title #=> "The title of the image"
+
+### Writing XML data
+
+    image.to_xml #=> <xmp data in xml format />
+
+### Creating an Image from scratch
+
+    image = AVM::Image.new
+    image.title = "The title of the image"
+    
+    observation = image.create_observation(:instrument => 'HST', :color_assignment => 'Green')
+    contact = image.creator.create_contact(:name => 'John Bintz')
+
+## Command line tool
+
+`avm2avm` currently performs one function: take an XMP file from stdin and pretty print the image as a Hash:
+
+    avm2avm < my-file.xmp
+
+## More resources
+
+* RDoc: http://ruby-docs.info/blah
+* AVM Standard: http://www.virtualastronomy.org/
+

lib/avm/cli.rb

@@ -3,6 +3,7 @@ require 'avm/image'
 require 'pp'
 
 module AVM
+  # The CLI interface
   class CLI < ::Thor
     default_task :convert
 

lib/avm/contact.rb

@@ -1,4 +1,5 @@
 module AVM
+  # A contributor to an image
   class Contact
     FIELD_MAP = {
       :zip => :postal_code,

lib/avm/controlled_vocabulary.rb

@@ -1,4 +1,5 @@
 module AVM
+  # Build a ControlledVocabulary set of classes for use with CV fields
   module ControlledVocabulary
     class << self
       def included(klass)

lib/avm/creator.rb

@@ -2,6 +2,7 @@ require 'avm/contact'
 require 'nokogiri'
 
 module AVM
+  # A container for Contacts (contributors to an image)
   class Creator
     attr_reader :contacts, :image
 
@@ -34,8 +35,8 @@ module AVM
     end
 
     def method_missing(key, *opts)
-      if key.to_s[-1..-1] == '='
+      if (key_to_s = key.to_s)[-1..-1] == '='
-        @options[key.to_s[0..-2].to_sym] = opts.first
+        @options[key_to_s[0..-2].to_sym] = opts.first
       else
         if PRIMARY_CONTACT_FIELDS.include?(key)
           primary_contact_field key

lib/avm/image.rb

@@ -8,6 +8,7 @@ require 'avm/coordinate_frame'
 require 'avm/observation'
 
 module AVM
+  # A single image, which has Observations, Contacts, and other metadata
   class Image
     DUBLIN_CORE_FIELDS = [ :title, :description ]
 
@@ -151,16 +152,15 @@ module AVM
       @options = options
 
       AVM_TO_FLOAT.each do |field| 
-        if @options[field]
+        @options[field] = case (value = @options[field])
-          case @options[field]
+                          when Array
-          when Array
+                            value.collect(&:to_f)
-            @options[field].collect!(&:to_f)
+                          else
-          else
+                            value ? value.to_f : nil
-            @options[field] = @options[field].to_f
+                          end
-          end
-        end
       end
 
+
       @observations = []
     end
 
@@ -336,11 +336,9 @@ module AVM
       end
 
       def string_date_or_nil(field)
-        return nil if !send(field)
+        (value = send(field)) ? value.strftime('%Y-%m-%d') : nil
-        send(field).strftime('%Y-%m-%d')
       end
 
-
       def alt_li_tag(text)
         %{<rdf:Alt><rdf:li xml:lang="x-default">#{text}</rdf:li></rdf:Alt>}
       end

lib/avm/node.rb

@@ -1,6 +1,7 @@
 require 'delegate'
 
 module AVM
+  # Delegate of Nokogiri::XML::Node which fixes XPath queries to use the correct namespace prefixes
   class Node < DelegateClass(Nokogiri::XML::Node)
     def initialize(xmp, node)
       @xmp, @node = xmp, node

lib/avm/observation.rb

@@ -1,4 +1,6 @@
 module AVM
+  # An individual observation by a single instrument w/ specific settings.
+  # Astronomical images are made of one or more Observations.
   class Observation
     AVM_SINGLE_FIELDS = %w{Facility Instrument Spectral.ColorAssignment Spectral.Band Spectral.Bandpass Spectral.CentralWavelength Temporal.StartTime Temporal.IntegrationTime DatasetID}
     AVM_SINGLE_METHODS = [ :facility, :instrument, :color_assignment, :band, :bandpass, :wavelength, :string_start_time, :integration_time, :dataset_id ]
@@ -7,9 +9,8 @@ module AVM
     attr_reader :image, :options
 
     def initialize(image, options = {})
-      options[:start_time] = options[:string_start_time] if options[:string_start_time]
-
       @image, @options = image, options
+      @options[:start_time] = @options[:string_start_time] || @options[:start_time]
     end
   
     def method_missing(method)
@@ -17,7 +18,7 @@ module AVM
     end
 
     def wavelength
-      @options[:wavelength] ? @options[:wavelength].to_f : nil
+      (wavelength = @options[:wavelength]) ? wavelength.to_f : nil
     end
 
     def start_time
@@ -25,11 +26,7 @@ module AVM
     end
 
     def string_start_time
-      if start_time
+      start_time ? start_time.strftime('%Y-%m-%dT%H:%M') : nil
-        start_time.strftime('%Y-%m-%dT%H:%M')
-      else
-        nil
-      end
     end
 
     def to_h

lib/avm/xmp.rb

@@ -2,6 +2,7 @@ require 'nokogiri'
 require 'avm/node'
 
 module AVM
+  # An XMP document wrapper, providing namespace handling and document reference assistance.
   class XMP
     PREFIXES = {
       'dc' => 'Dublin Core',
@@ -58,20 +59,26 @@ module AVM
     end
 
     private
-      def prefix_map
+      def current_namespaces
-        @prefix_map ||= Hash[doc.document.collect_namespaces.collect { |prefix, namespace|
+        doc.document.collect_namespaces
-          prefix = prefix.gsub('xmlns:', '')
+      end
-          result = nil
 
-          REQUIRED_NAMESPACES.each do |original_prefix, target_namespace|
+      def prefix_map
-            result = [ original_prefix.to_s, prefix ] if namespace == target_namespace
+        @prefix_map ||= Hash[current_namespaces.collect { |prefix, namespace| 
-          end
+          self.class.get_required_namespace(namespace, prefix.gsub('xmlns:', '')) 
-          result
         }.compact]
       end
+      
+      def self.get_required_namespace(namespace, prefix)
+        result = nil
+        REQUIRED_NAMESPACES.each do |original_prefix, target_namespace|
+          result = [ original_prefix.to_s, prefix ] if namespace == target_namespace
+        end
+        result
+      end
 
       def ensure_namespaces!
-        existing = doc.document.collect_namespaces
+        existing = current_namespaces
 
         REQUIRED_NAMESPACES.each do |namespace, url|
           doc.root.add_namespace_definition(namespace.to_s, url) if !existing.values.include?(url)
@@ -83,8 +90,8 @@ module AVM
 
         search('//rdf:Description').each do |description|
           if first_child = description.first_element_child
-            if first_child.namespace
+            if namespace = first_child.namespace
-              prefix = first_child.namespace.prefix
+              prefix = namespace.prefix
 
               if prefix_description = PREFIXES[prefix_map.index(prefix)]
                 description[self % 'rdf:about'] = prefix_description
@@ -94,13 +101,17 @@ module AVM
           end
         end
 
-        if !at_xpath('//rdf:RDF')
+        ensure_rdf!
-          doc.first_element_child.add_child(self % '<rdf:RDF />')
+        ensure_missing_descriptions!(added)
-        end
+      end
 
+      def ensure_rdf!
+        doc.first_element_child.add_child(self % '<rdf:RDF />') if !at_xpath('//rdf:RDF')
+      end
 
+      def ensure_missing_descriptions!(already_added)
         PREFIXES.each do |prefix, about|
-          if !added.include?(prefix)
+          if !already_added.include?(prefix)
             at_xpath('//rdf:RDF').add_child(self % %{<rdf:Description rdf:about="#{about}" />})
           end
         end

reek.watchr

@@ -0,0 +1,12 @@
+watch('lib/(.*)\.rb') { |file| reek(file[0]) }
+watch('spec/(.*)_spec\.rb') { |file| reek("lib/#{file[1]}.rb") }
+
+def reek(file = nil)
+  file ||= Dir['lib/**/*.rb'].join(' ')
+  spec_file = file.gsub('lib/', 'spec/').gsub('.rb', '_spec.rb')
+
+  system %{bundle exec rspec -c #{spec_file}}
+  system %{reek #{file}}
+end
+
+reek

spec/avm/image_spec.rb

@@ -290,15 +290,15 @@ describe AVM::Image do
       it "should have the spatial tags" do
         xpath_text(avm, './avm:Spatial.CoordinateFrame').should == coordinate_frame
         xpath_text(avm, './avm:Spatial.Equinox').should == equinox
-        xpath_list(avm, './avm:Spatial.ReferenceValue').should == reference_value.collect(&:to_s)
+        xpath_list(avm, './avm:Spatial.ReferenceValue').should == reference_value.collect { |v| v.to_f.to_s }
-        xpath_list(avm, './avm:Spatial.ReferenceDimension').should == reference_dimension.collect(&:to_s)
+        xpath_list(avm, './avm:Spatial.ReferenceDimension').should == reference_dimension.collect { |v| v.to_f.to_s }
-        xpath_list(avm, './avm:Spatial.ReferencePixel').should == reference_pixel.collect(&:to_s)
+        xpath_list(avm, './avm:Spatial.ReferencePixel').should == reference_pixel.collect { |v| v.to_f.to_s }
-        xpath_list(avm, './avm:Spatial.Scale').should == spatial_scale.collect(&:to_s)
+        xpath_list(avm, './avm:Spatial.Scale').should == spatial_scale.collect { |v| v.to_f.to_s }
         xpath_text(avm, './avm:Spatial.CoordsystemProjection').should == coordinate_system_projection
         xpath_text(avm, './avm:Spatial.Quality').should == spatial_quality
         xpath_text(avm, './avm:Spatial.Notes').should == spatial_notes
         xpath_text(avm, './avm:Spatial.FITSheader').should == fits_header
-        xpath_list(avm, './avm:Spatial.CDMatrix').should == spatial_cd_matrix.collect(&:to_s)
+        xpath_list(avm, './avm:Spatial.CDMatrix').should == spatial_cd_matrix.collect { |v| v.to_f.to_s }
       end
 
       it "should have the publisher tags" do