Martin Thompson has proposed merging lp:~mjthompson/openlp/opensong_import into lp:openlp.
Requested reviews: Raoul Snyman (raoul-snyman) Tim Bentley (trb143) Integrated with the song import wizard. More testcases with synthetic data. More testing done with more real data. -- https://code.launchpad.net/~mjthompson/openlp/opensong_import/+merge/35581 Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/plugins/songs/lib/opensongimport.py' --- openlp/plugins/songs/lib/opensongimport.py 2010-09-14 14:21:44 +0000 +++ openlp/plugins/songs/lib/opensongimport.py 2010-09-15 20:17:43 +0000 @@ -29,6 +29,7 @@ from zipfile import ZipFile from lxml import objectify from lxml.etree import Error, LxmlError +import re from openlp.core.lib import translate from openlp.plugins.songs.lib.songimport import SongImport @@ -113,12 +114,22 @@ def do_import(self): """ - Import either a single opensong file, or a zipfile containing multiple - opensong files. If `self.commit` is set False, the import will not be - committed to the database (useful for test scripts). + Import either each of the files in self.filenames - each element of + which can be either a single opensong file, or a zipfile containing + multiple opensong files. If `self.commit` is set False, the + import will not be committed to the database (useful for test scripts). """ success = True - self.import_wizard.importProgressBar.setMaximum(len(self.filenames)) + numfiles = 0 + for filename in self.filenames: + ext = os.path.splitext(filename)[1] + if ext.lower() == u'.zip': + z = ZipFile(filename, u'r') + numfiles += len(z.infolist()) + else: + numfiles += 1 + log.debug("Total number of files: %d", numfiles) + self.import_wizard.importProgressBar.setMaximum(numfiles) for filename in self.filenames: if self.stop_import_flag: success = False @@ -127,9 +138,6 @@ if ext.lower() == u'.zip': log.debug(u'Zipfile found %s', filename) z = ZipFile(filename, u'r') - self.import_wizard.importProgressBar.setMaximum( - self.import_wizard.importProgressBar.maximum() + - len(z.infolist())) for song in z.infolist(): if self.stop_import_flag: success = False @@ -138,6 +146,7 @@ if parts[-1] == u'': #No final part => directory continue + log.info(u'Zip importing %s', parts[-1]) self.import_wizard.incrementProgressBar( unicode(translate('SongsPlugin.ImportWizardForm', 'Importing %s...')) % parts[-1]) @@ -145,11 +154,11 @@ self.do_import_file(songfile) if self.commit: self.finish() - self.set_defaults() if self.stop_import_flag: success = False break else: + # not a zipfile log.info('Direct import %s', filename) self.import_wizard.incrementProgressBar( unicode(translate('SongsPlugin.ImportWizardForm', @@ -158,9 +167,7 @@ self.do_import_file(file) if self.commit: self.finish() - self.set_defaults() - if not self.commit: - self.finish() + return success def do_import_file(self, file): @@ -168,7 +175,7 @@ Process the OpenSong file - pass in a file-like object, not a filename """ - self.authors = [] + self.set_defaults() try: tree = objectify.parse(file) except (Error, LxmlError): @@ -197,7 +204,6 @@ self.topics.append(unicode(root.alttheme)) # data storage while importing verses = {} - lyrics = unicode(root.lyrics) # keep track of a "default" verse order, in case none is specified our_verse_order = [] verses_seen = {} @@ -205,6 +211,7 @@ # erm, versetype! versetype = u'V' versenum = None + lyrics = unicode(root.lyrics) for thisline in lyrics.split(u'\n'): # remove comments semicolon = thisline.find(u';') @@ -219,16 +226,18 @@ continue # verse/chorus/etc. marker if thisline[0] == u'[': - versetype = thisline[1].upper() - if versetype.isdigit(): - versenum = versetype - versetype = u'V' - elif thisline[2] != u']': - # there's a number to go with it - extract that as well - right_bracket = thisline.find(u']') - versenum = thisline[2:right_bracket] + # drop the square brackets + right_bracket = thisline.find(u']') + content = thisline[1:right_bracket].upper() + # have we got any digits? If so, versenumber is everything from the digits + # to the end (even if there are some alpha chars on the end) + match = re.match(u'(.*)(\d+.*)', content) + if match is not None: + versetype = match.group(1) + versenum = match.group(2) + # otherwise we assume number 1 and take the whole prefix as versetype else: - # if there's no number, assume it's no.1 + versetype = content versenum = u'1' continue words = None @@ -236,10 +245,10 @@ if thisline[0].isdigit(): versenum = thisline[0] words = thisline[1:].strip() - if words is None and \ - versenum is not None and \ - versetype is not None: + if words is None: words = thisline + if not versenum: + versenum = u'1' if versenum is not None: versetag = u'%s%s' % (versetype, versenum) if not verses.has_key(versetype): @@ -260,10 +269,13 @@ versetypes.sort() versetags = {} for versetype in versetypes: + our_verse_type = versetype + if our_verse_type == u'': + our_verse_type = u'V' versenums = verses[versetype].keys() versenums.sort() for num in versenums: - versetag = u'%s%s' % (versetype, num) + versetag = u'%s%s' % (our_verse_type, num) lines = u'\n'.join(verses[versetype][num]) self.verses.append([versetag, lines]) # Keep track of what we have for error checking later @@ -272,17 +284,27 @@ order = [] if u'presentation' in fields and root.presentation != u'': order = unicode(root.presentation) - order = order.split() + # We make all the tags in the lyrics upper case, so match that here + # and then split into a list on the whitespace + order = order.upper().split() else: if len(our_verse_order) > 0: order = our_verse_order else: +<<<<<<< TREE log.warn(u'No verse order available for %s, skipping.', self.title) +======= + log.warn(u'No verse order available (either explicit or inferred) for %s, skipping.', self.title) +>>>>>>> MERGE-SOURCE for tag in order: - if len(tag) == 1: - tag = tag + u'1' # Assume it's no.1 if it's not there + if tag[0].isdigit(): + # Assume it's a verse if it has no prefix + tag = u'V' + tag + elif not re.search('\d+', tag): + # Assume it's no.1 if there's no digits + tag = tag + u'1' if not versetags.has_key(tag): - log.warn(u'Got order %s but not in versetags, skipping', tag) + log.info(u'Got order %s but not in versetags, dropping this item from presentation order', tag) else: self.verse_order_list.append(tag) === modified file 'openlp/plugins/songs/lib/songimport.py' --- openlp/plugins/songs/lib/songimport.py 2010-09-09 19:34:45 +0000 +++ openlp/plugins/songs/lib/songimport.py 2010-09-15 20:17:43 +0000 @@ -55,8 +55,12 @@ self.set_defaults() QtCore.QObject.connect(Receiver.get_receiver(), QtCore.SIGNAL(u'songs_stop_import'), self.stop_import) - def set_defaults(self): + """ + Create defaults for properties - call this before each song + if importing many songs at once to ensure a clean beginning + """ + self.authors = [] self.title = u'' self.song_number = u'' self.alternate_title = u'' @@ -255,13 +259,16 @@ """ Write the song and its fields to disk """ + log.info(u'commiting song %s to database', self.title) song = Song() song.title = self.title song.search_title = self.remove_punctuation(self.title) \ + '@' + self.alternate_title song.song_number = self.song_number song.search_lyrics = u'' + verses_changed_to_other = {} sxml = SongXMLBuilder() + other_count = 1 for (versetag, versetext) in self.verses: if versetag[0] == u'C': versetype = VerseType.to_string(VerseType.Chorus) @@ -276,10 +283,18 @@ elif versetag[0] == u'E': versetype = VerseType.to_string(VerseType.Ending) else: + newversetag = u'O%d' % other_count + verses_changed_to_other[versetag] = newversetag + other_count += 1 versetype = VerseType.to_string(VerseType.Other) + log.info(u'Versetype %s changing to %s' , versetag, newversetag) + versetag = newversetag sxml.add_verse_to_lyrics(versetype, versetag[1:], versetext) song.search_lyrics += u' ' + self.remove_punctuation(versetext) song.lyrics = unicode(sxml.extract_xml(), u'utf-8') + for i, current_verse_tag in enumerate(self.verse_order_list): + if verses_changed_to_other.has_key(current_verse_tag): + self.verse_order_list[i] = verses_changed_to_other[current_verse_tag] song.verse_order = u' '.join(self.verse_order_list) song.copyright = self.copyright song.comments = self.comments === modified file 'openlp/plugins/songs/lib/test/test.opensong' --- openlp/plugins/songs/lib/test/test.opensong 2010-07-09 18:08:03 +0000 +++ openlp/plugins/songs/lib/test/test.opensong 2010-09-15 20:17:43 +0000 @@ -1,10 +1,10 @@ <?xml version="1.0" encoding="UTF-8"?> <song> <title>Martins Test</title> - <author>Martià Thómpson</author> + <author>Martià & Martin2 Thómpson</author> <copyright>2010 Martin Thompson</copyright> <hymn_number>1</hymn_number> - <presentation>V1 C V2 C2 V3 B1 V1</presentation> + <presentation>V1 C V2 C2 3a B1 V1 T U Rap1 Rap2 Rap3</presentation> <ccli>Blah</ccli> <capo print="false"></capo> <key></key> @@ -17,7 +17,12 @@ <alttheme>TestAltTheme</alttheme> <tempo></tempo> <time_sig></time_sig> - <lyrics>;Comment + <lyrics>[3a] +. G A B + V3 Line 1 +. G A B + V3 Line 2 + . A B C 1 v1 Line 1___ 2 v2 Line 1___ @@ -25,10 +30,6 @@ 1 V1 Line 2 2 V2 Line 2 -[3] - V3 Line 1 - V3 Line 2 - [b1] Bridge 1 --- @@ -36,12 +37,29 @@ Bridge 1 line 2 [C] -. A B + . A B Chorus 1 [C2] . A B Chorus 2 + +[T] + T Line 1 + +[Rap] +1 Rap 1 Line 1 +2 Rap 2 Line 1 +1 Rap 1 Line 2 +2 Rap 2 Line 2 + +[rap3] + Rap 3 Line 1 + Rap 3 Line 2 + + +[X] + Unreferenced verse line 1 </lyrics> <style index="default_style"> <title enabled="true" valign="bottom" align="center" include_verse="false" margin-left="0" margin-right="0" margin-top="0" margin-bottom="0" font="Helvetica" size="26" bold="true" italic="true" underline="false" color="#FFFFFF" border="true" border_color="#000000" shadow="true" shadow_color="#000000" fill="false" fill_color="#000000"/> === added file 'openlp/plugins/songs/lib/test/test3.opensong' --- openlp/plugins/songs/lib/test/test3.opensong 1970-01-01 00:00:00 +0000 +++ openlp/plugins/songs/lib/test/test3.opensong 2010-09-15 20:17:43 +0000 @@ -0,0 +1,10 @@ +<?xml version="1.0" encoding="UTF-8"?> +<song> + <title>Test single verse</title> + <author>Martin Thompson</author> + <copyright>2010</copyright> + <ccli>123456</ccli> + <theme>Worship: Declaration</theme> + <lyrics> Line 1 +Line 2 +</lyrics></song> === added file 'openlp/plugins/songs/lib/test/test_import_file.py' --- openlp/plugins/songs/lib/test/test_import_file.py 1970-01-01 00:00:00 +0000 +++ openlp/plugins/songs/lib/test/test_import_file.py 2010-09-15 20:17:43 +0000 @@ -0,0 +1,47 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=80 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2010 Raoul Snyman # +# Portions copyright (c) 2008-2010 Tim Bentley, Jonathan Corwin, Michael # +# Gorven, Scott Guerrieri, Meinert Jordan, Andreas Preikschat, Christian # +# Richter, Philip Ridout, Maikel Stuivenberg, Martin Thompson, Jon Tibble, # +# Carsten Tinggaard, Frode Woldsund # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +import sys + +from openlp.plugins.songs.lib.opensongimport import OpenSongImport +from openlp.core.lib.db import Manager +from openlp.plugins.songs.lib.db import init_schema + +import logging +LOG_FILENAME = 'test_import_file.log' +logging.basicConfig(filename=LOG_FILENAME,level=logging.INFO) + +from test_opensongimport import wizard_stub, progbar_stub + +def test(filenames): + manager = Manager(u'songs', init_schema) + o = OpenSongImport(manager, filenames=filenames) + o.import_wizard = wizard_stub() + o.commit = False + o.do_import() + o.print_song() + +if __name__ == "__main__": + test(sys.argv[1:]) === modified file 'openlp/plugins/songs/lib/test/test_importing_lots.py' --- openlp/plugins/songs/lib/test/test_importing_lots.py 2010-07-15 20:29:24 +0000 +++ openlp/plugins/songs/lib/test/test_importing_lots.py 2010-09-15 20:17:43 +0000 @@ -8,50 +8,29 @@ import sys import codecs +import logging +LOG_FILENAME = 'import.log' +logging.basicConfig(filename=LOG_FILENAME,level=logging.INFO) + +from test_opensongimport import wizard_stub, progbar_stub + +# Useful test function for importing a variety of different files +# Uncomment below depending on what problem trying to make occur! + def opensong_import_lots(): ziploc = u'/home/mjt/openlp/OpenSong_Data/' files = [] - #files = [u'test.opensong.zip', ziploc+u'ADond.zip'] - files.extend(glob(ziploc+u'Songs.zip')) - #files.extend(glob(ziploc+u'SOF.zip')) - #files.extend(glob(ziploc+u'spanish_songs_for_opensong.zip')) -# files.extend(glob(ziploc+u'opensong_*.zip')) + files = [os.path.join(ziploc, u'RaoulSongs', u'Songs', u'Jesus Freak')] + # files.extend(glob(ziploc+u'Songs.zip')) + # files.extend(glob(ziploc+u'RaoulSongs.zip')) + # files.extend(glob(ziploc+u'SOF.zip')) + # files.extend(glob(ziploc+u'spanish_songs_for_opensong.zip')) + # files.extend(glob(ziploc+u'opensong_*.zip')) errfile = codecs.open(u'import_lots_errors.txt', u'w', u'utf8') manager = Manager(u'songs', init_schema) - for file in files: - print u'Importing', file - z = ZipFile(file, u'r') - for song in z.infolist(): - # need to handle unicode filenames (CP437 - Winzip does this) - filename = song.filename#.decode('cp852') - parts = os.path.split(filename) - if parts[-1] == u'': - #No final part => directory - continue - print " ", file, ":",filename, - songfile = z.open(song) - #z.extract(song) - #songfile=open(filename, u'r') - o = OpenSongImport(manager) - try: - o.do_import_file(songfile) - # o.song_import.print_song() - except: - print "Failure", - - errfile.write(u'Failure: %s:%s\n' %(file, filename.decode('cp437'))) - songfile = z.open(song) - for l in songfile.readlines(): - l = l.decode('utf8') - print(u' |%s\n' % l.strip()) - errfile.write(u' |%s\n'%l.strip()) - print_exc(3, file = errfile) - print_exc(3) - sys.exit(1) - # continue - #o.finish() - print "OK" - #os.unlink(filename) - # o.song_import.print_song() + o = OpenSongImport(manager, filenames=files) + o.import_wizard=wizard_stub() + o.do_import() + if __name__ == "__main__": opensong_import_lots() === modified file 'openlp/plugins/songs/lib/test/test_opensongimport.py' --- openlp/plugins/songs/lib/test/test_opensongimport.py 2010-07-27 09:32:52 +0000 +++ openlp/plugins/songs/lib/test/test_opensongimport.py 2010-09-15 20:17:43 +0000 @@ -28,62 +28,101 @@ from openlp.core.lib.db import Manager from openlp.plugins.songs.lib.db import init_schema +import logging +LOG_FILENAME = 'test.log' +logging.basicConfig(filename=LOG_FILENAME,level=logging.INFO) + +# Stubs to replace the UI functions for raw testing +class wizard_stub: + def __init__(self): + self.importProgressBar=progbar_stub() + def incrementProgressBar(self, str): + pass +class progbar_stub: + def __init__(self): + pass + def setMaximum(self, arg): + pass + def test(): manager = Manager(u'songs', init_schema) - o = OpenSongImport(manager) - o.do_import(u'test.opensong', commit=False) - o.song_import.print_song() - assert o.song_import.copyright == u'2010 Martin Thompson' - assert o.song_import.authors == [u'Martià Thómpson'] - assert o.song_import.title == u'Martins Test' - assert o.song_import.alternate_title == u'' - assert o.song_import.song_number == u'1' - assert [u'C1', u'Chorus 1'] in o.song_import.verses - assert [u'C2', u'Chorus 2'] in o.song_import.verses - assert not [u'C3', u'Chorus 3'] in o.song_import.verses - assert [u'B1', u'Bridge 1\nBridge 1 line 2'] in o.song_import.verses - assert [u'V1', u'v1 Line 1\nV1 Line 2'] in o.song_import.verses - assert [u'V2', u'v2 Line 1\nV2 Line 2'] in o.song_import.verses - assert o.song_import.verse_order_list == [u'V1', u'C1', u'V2', u'C2', u'V3', u'B1', u'V1'] - assert o.song_import.ccli_number == u'Blah' - assert o.song_import.topics == [u'TestTheme', u'TestAltTheme'] - o.do_import(u'test.opensong.zip', commit=False) - o.song_import.print_song() - o.finish() - assert o.song_import.copyright == u'2010 Martin Thompson' - assert o.song_import.authors == [u'Martià Thómpson'] - assert o.song_import.title == u'Martins Test' - assert o.song_import.alternate_title == u'' - assert o.song_import.song_number == u'1' - assert [u'B1', u'Bridge 1\nBridge 1 line 2'] in o.song_import.verses - assert [u'C1', u'Chorus 1'] in o.song_import.verses - assert [u'C2', u'Chorus 2'] in o.song_import.verses - assert not [u'C3', u'Chorus 3'] in o.song_import.verses - assert [u'V1', u'v1 Line 1\nV1 Line 2'] in o.song_import.verses - assert [u'V2', u'v2 Line 1\nV2 Line 2'] in o.song_import.verses - assert o.song_import.verse_order_list == [u'V1', u'C1', u'V2', u'C2', u'V3', u'B1', u'V1'] - - o = OpenSongImport(manager) - o.do_import(u'test2.opensong', commit=False) - # o.finish() - o.song_import.print_song() - assert o.song_import.copyright == u'2010 Martin Thompson' - assert o.song_import.authors == [u'Martin Thompson'] - assert o.song_import.title == u'Martins 2nd Test' - assert o.song_import.alternate_title == u'' - assert o.song_import.song_number == u'2' - print o.song_import.verses - assert [u'B1', u'Bridge 1\nBridge 1 line 2'] in o.song_import.verses - assert [u'C1', u'Chorus 1'] in o.song_import.verses - assert [u'C2', u'Chorus 2'] in o.song_import.verses - assert not [u'C3', u'Chorus 3'] in o.song_import.verses - assert [u'V1', u'v1 Line 1\nV1 Line 2'] in o.song_import.verses - assert [u'V2', u'v2 Line 1\nV2 Line 2'] in o.song_import.verses - print o.song_import.verse_order_list - assert o.song_import.verse_order_list == [u'V1', u'V2', u'B1', u'C1', u'C2'] + o = OpenSongImport(manager, filenames=[u'test.opensong']) + o.import_wizard = wizard_stub() + o.commit = False + o.do_import() + o.print_song() + assert o.copyright == u'2010 Martin Thompson' + assert o.authors == [u'Martià Thómpson', u'Martin2 Thómpson'] + assert o.title == u'Martins Test' + assert o.alternate_title == u'' + assert o.song_number == u'1' + assert [u'C1', u'Chorus 1'] in o.verses + assert [u'C2', u'Chorus 2'] in o.verses + assert not [u'C3', u'Chorus 3'] in o.verses + assert [u'B1', u'Bridge 1\nBridge 1 line 2'] in o.verses + assert [u'V1', u'v1 Line 1\nV1 Line 2'] in o.verses + assert [u'V2', u'v2 Line 1\nV2 Line 2'] in o.verses + assert [u'V3A', u'V3 Line 1\nV3 Line 2'] in o.verses + assert [u'RAP1', u'Rap 1 Line 1\nRap 1 Line 2'] in o.verses + assert [u'RAP2', u'Rap 2 Line 1\nRap 2 Line 2'] in o.verses + assert [u'RAP3', u'Rap 3 Line 1\nRap 3 Line 2'] in o.verses + assert [u'X1', u'Unreferenced verse line 1'] in o.verses + assert o.verse_order_list == [u'V1', u'C1', u'V2', u'C2', u'V3A', u'B1', u'V1', u'T1', u'RAP1', u'RAP2', u'RAP3'] + assert o.ccli_number == u'Blah' + assert o.topics == [u'TestTheme', u'TestAltTheme'] + + o.filenames = [u'test.opensong.zip'] + o.set_defaults() + o.do_import() + o.print_song() + assert o.copyright == u'2010 Martin Thompson' + assert o.authors == [u'Martià Thómpson'] + assert o.title == u'Martins Test' + assert o.alternate_title == u'' + assert o.song_number == u'1' + assert [u'B1', u'Bridge 1\nBridge 1 line 2'] in o.verses + assert [u'C1', u'Chorus 1'] in o.verses + assert [u'C2', u'Chorus 2'] in o.verses + assert not [u'C3', u'Chorus 3'] in o.verses + assert [u'V1', u'v1 Line 1\nV1 Line 2'] in o.verses + assert [u'V2', u'v2 Line 1\nV2 Line 2'] in o.verses + print o.verse_order_list + assert o.verse_order_list == [u'V1', u'C1', u'V2', u'C2', u'V3', u'B1', u'V1'] + + o.filenames = [u'test2.opensong'] + o.set_defaults() + o.do_import() + o.print_song() + assert o.copyright == u'2010 Martin Thompson' + assert o.authors == [u'Martin Thompson'] + assert o.title == u'Martins 2nd Test' + assert o.alternate_title == u'' + assert o.song_number == u'2' + print o.verses + assert [u'B1', u'Bridge 1\nBridge 1 line 2'] in o.verses + assert [u'C1', u'Chorus 1'] in o.verses + assert [u'C2', u'Chorus 2'] in o.verses + assert not [u'C3', u'Chorus 3'] in o.verses + assert [u'V1', u'v1 Line 1\nV1 Line 2'] in o.verses + assert [u'V2', u'v2 Line 1\nV2 Line 2'] in o.verses + print o.verse_order_list + assert o.verse_order_list == [u'V1', u'V2', u'B1', u'C1', u'C2'] + + o.filenames = [u'test3.opensong'] + o.set_defaults() + o.do_import() + o.print_song() + assert o.copyright == u'2010' + assert o.authors == [u'Martin Thompson'] + assert o.title == u'Test single verse' + assert o.alternate_title == u'' + assert o.ccli_number == u'123456' + assert o.verse_order_list == [u'V1'] + assert o.topics == [u'Worship: Declaration'] + print o.verses[0] + assert [u'V1', u'Line 1\nLine 2'] in o.verses print "Tests passed" - pass if __name__ == "__main__": test()
_______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp