From 8c65729a511c3a3cdc6362ad267f9cbe38c27b43 Mon Sep 17 00:00:00 2001 From: John Bintz Date: Wed, 23 Mar 2011 14:10:02 -0400 Subject: [PATCH] code cleanup thanks to reek --- README.md | 57 +++++++++++++++++++++++++++++++- lib/avm/cli.rb | 1 + lib/avm/contact.rb | 1 + lib/avm/controlled_vocabulary.rb | 1 + lib/avm/creator.rb | 5 +-- lib/avm/image.rb | 20 +++++------ lib/avm/node.rb | 1 + lib/avm/observation.rb | 13 +++----- lib/avm/xmp.rb | 41 ++++++++++++++--------- reek.watchr | 12 +++++++ spec/avm/image_spec.rb | 10 +++--- 11 files changed, 120 insertions(+), 42 deletions(-) create mode 100644 reek.watchr diff --git a/README.md b/README.md index 2f4633f..b9d1af8 100644 --- a/README.md +++ b/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 #=> + +### 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/ + diff --git a/lib/avm/cli.rb b/lib/avm/cli.rb index bf18cee..4823fe3 100644 --- a/lib/avm/cli.rb +++ b/lib/avm/cli.rb @@ -3,6 +3,7 @@ require 'avm/image' require 'pp' module AVM + # The CLI interface class CLI < ::Thor default_task :convert diff --git a/lib/avm/contact.rb b/lib/avm/contact.rb index 852098a..8691242 100644 --- a/lib/avm/contact.rb +++ b/lib/avm/contact.rb @@ -1,4 +1,5 @@ module AVM + # A contributor to an image class Contact FIELD_MAP = { :zip => :postal_code, diff --git a/lib/avm/controlled_vocabulary.rb b/lib/avm/controlled_vocabulary.rb index 34f5113..e820398 100644 --- a/lib/avm/controlled_vocabulary.rb +++ b/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) diff --git a/lib/avm/creator.rb b/lib/avm/creator.rb index cadea35..3336f2a 100644 --- a/lib/avm/creator.rb +++ b/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] == '=' - @options[key.to_s[0..-2].to_sym] = opts.first + if (key_to_s = key.to_s)[-1..-1] == '=' + @options[key_to_s[0..-2].to_sym] = opts.first else if PRIMARY_CONTACT_FIELDS.include?(key) primary_contact_field key diff --git a/lib/avm/image.rb b/lib/avm/image.rb index 1e95fb3..6cb1a4a 100644 --- a/lib/avm/image.rb +++ b/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] - case @options[field] - when Array - @options[field].collect!(&:to_f) - else - @options[field] = @options[field].to_f - end - end + @options[field] = case (value = @options[field]) + when Array + value.collect(&:to_f) + else + value ? value.to_f : nil + end end + @observations = [] end @@ -336,11 +336,9 @@ module AVM end def string_date_or_nil(field) - return nil if !send(field) - send(field).strftime('%Y-%m-%d') + (value = send(field)) ? value.strftime('%Y-%m-%d') : nil end - def alt_li_tag(text) %{<rdf:Alt><rdf:li xml:lang="x-default">#{text}</rdf:li></rdf:Alt>} end diff --git a/lib/avm/node.rb b/lib/avm/node.rb index 465a265..0f9e98d 100644 --- a/lib/avm/node.rb +++ b/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 diff --git a/lib/avm/observation.rb b/lib/avm/observation.rb index e5ccea9..a893319 100644 --- a/lib/avm/observation.rb +++ b/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.strftime('%Y-%m-%dT%H:%M') - else - nil - end + start_time ? start_time.strftime('%Y-%m-%dT%H:%M') : nil end def to_h diff --git a/lib/avm/xmp.rb b/lib/avm/xmp.rb index 50d763f..533356a 100644 --- a/lib/avm/xmp.rb +++ b/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 - @prefix_map ||= Hash[doc.document.collect_namespaces.collect { |prefix, namespace| - prefix = prefix.gsub('xmlns:', '') - result = nil + def current_namespaces + doc.document.collect_namespaces + end - REQUIRED_NAMESPACES.each do |original_prefix, target_namespace| - result = [ original_prefix.to_s, prefix ] if namespace == target_namespace - end - result + def prefix_map + @prefix_map ||= Hash[current_namespaces.collect { |prefix, namespace| + self.class.get_required_namespace(namespace, prefix.gsub('xmlns:', '')) }.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 - prefix = first_child.namespace.prefix + if namespace = first_child.namespace + 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') - doc.first_element_child.add_child(self % '<rdf:RDF />') - end + ensure_rdf! + ensure_missing_descriptions!(added) + 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 diff --git a/reek.watchr b/reek.watchr new file mode 100644 index 0000000..0a6a073 --- /dev/null +++ b/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 diff --git a/spec/avm/image_spec.rb b/spec/avm/image_spec.rb index 2d6bc30..2a36719 100644 --- a/spec/avm/image_spec.rb +++ b/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.ReferenceDimension').should == reference_dimension.collect(&:to_s) - xpath_list(avm, './avm:Spatial.ReferencePixel').should == reference_pixel.collect(&:to_s) - xpath_list(avm, './avm:Spatial.Scale').should == spatial_scale.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 { |v| v.to_f.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 { |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