Description
flanagan
2011-04-28 13:39:12 UTC
I'm taking this on now. A work-in-progress branch that replaces the current sort routine with one using Library::CallNumber::LC can be found at https://github.com/gmcharlt/koha-test/commits/fix_lcc_cn_sort Additional examples for the test case would be welcome. Created attachment 14478 [details] [review] bug 6281: add test case for sorting LC call numbers correctly My thanks to Michael Flanagan of UCAR for providing some of these examples. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Created attachment 14479 [details] [review] bug 6281: introduce LC::CallNumber::LC for sorting LC call numbers This replaces the previous hand-coded normalizer. Because LC::CallNumber::LC appears to reject strings that aren't valid LC call numbers, significant changes to the test cases were made as well -- however, the one that really counts is the last one which verifies the sorting. To recalculate the call number sort key for each item, it is necessary to run misc/maintenance/touch_all_items.pl To test, create item records with the following call numbers, setting the classification sort to 'lccl: QC100 .U57 NO. 555 1986 QC145 .A57 V.12 1980 QC145.45 .H4 D65 1998 QC995 .E29 1997 Next, make a report of them in the inventory tool. The items should be sorted in the above order. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Created attachment 14480 [details] [review] bug 6281: add Library::CallNumber::LC as a required Perl dependency This module is currently packaged by Debian for Wheezy and by Ubuntu for Precise and Quantal. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Created attachment 14714 [details] [review] bug 6281: add test case for sorting LC call numbers correctly My thanks to Michael Flanagan of UCAR for providing some of these examples. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 14715 [details] [review] bug 6281: introduce LC::CallNumber::LC for sorting LC call numbers This replaces the previous hand-coded normalizer. Because LC::CallNumber::LC appears to reject strings that aren't valid LC call numbers, significant changes to the test cases were made as well -- however, the one that really counts is the last one which verifies the sorting. To recalculate the call number sort key for each item, it is necessary to run misc/maintenance/touch_all_items.pl To test, create item records with the following call numbers, setting the classification sort to 'lccl: QC100 .U57 NO. 555 1986 QC145 .A57 V.12 1980 QC145.45 .H4 D65 1998 QC995 .E29 1997 Next, make a report of them in the inventory tool. The items should be sorted in the above order. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 14716 [details] [review] bug 6281: add Library::CallNumber::LC as a required Perl dependency This module is currently packaged by Debian for Wheezy and by Ubuntu for Precise and Quantal. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> I set the rule "Default filing rules for LCC" for the classification source with the 'lcc' code. I created some items for one biblio with different callnumbers, run the maintenance script with the --where param (biblionumber=XXX). I created an inventory with callnumber from QC100 .U57 NO. 555 1986 to QC995 .E29 1997 and it returns items with the following order: QC100 .U57 NO. 555 1986 QC145.45 .H4 D65 1998 QC145 .A57 V.12 1980 QC20.45 .H4 D65 1998 QC995 .E29 1997 It seems that QC145.45 .H4 D65 1998 and QC145 .A57 V.12 1980 are inverted. What I missed? Perhaps I didn't understand what you meant by "setting the classification sort to 'lccl'". 'lccl' is a typo ? Changing status to Failed QA for the need of clarification (see previous comment). Thanks for your attention. Created attachment 18527 [details] [review] bug 6281: add test case for sorting LC call numbers correctly My thanks to Michael Flanagan of UCAR for providing some of these examples. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 18528 [details] [review] bug 6281: introduce LC::CallNumber::LC for sorting LC call numbers This replaces the previous hand-coded normalizer. Because LC::CallNumber::LC appears to reject strings that aren't valid LC call numbers, significant changes to the test cases were made as well -- however, the one that really counts is the last one which verifies the sorting. To recalculate the call number sort key for each item, it is necessary to run misc/maintenance/touch_all_items.pl To test, create item records with the following call numbers, setting the classification sort to 'lcc': QC100 .U57 NO. 555 1986 QC145 .A57 V.12 1980 QC145.45 .H4 D65 1998 QC995 .E29 1997 Next, make a report of them in the inventory tool. The items should be sorted in the above order. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 18529 [details] [review] bug 6281: add Library::CallNumber::LC as a required Perl dependency This module is currently packaged by Debian for Wheezy and by Ubuntu for Precise and Quantal. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> (In reply to comment #8) > I created an inventory with callnumber from QC100 .U57 NO. 555 1986 to QC995 > .E29 1997 and it returns items with the following order: > QC100 .U57 NO. 555 1986 > QC145.45 .H4 D65 1998 > QC145 .A57 V.12 1980 > QC20.45 .H4 D65 1998 > QC995 .E29 1997 > > It seems that QC145.45 .H4 D65 1998 and QC145 .A57 V.12 1980 are inverted. > > What I missed? I'm not seeing the problem. If you retest, please confirm that the items actually have cn_source set to 'lcc'. If you continue to run into the problem, please post the output of an SQL query that lists cn_source, cn_sort, and itemcallnumber for the relevant items. > Perhaps I didn't understand what you meant by "setting the classification > sort to 'lccl'". 'lccl' is a typo ? I have updated the patch series to fix the typo in the commit message and a trivial merge conflict in C4/PerlDepencies.pm, and set the status back to signed off. not ok 10 - LC call numbers sorted in correct order # Failed test 'LC call numbers sorted in correct order' # at t/ClassSortRoutine_LCC.t line 45. # Structures begin differing at: # $got->[1] = 'QC145.45 .H4 D65 1998' # $expected->[1] = 'QC145 .A57 V.12 1980' # Looks like you failed 1 test of 10. With the new tests, before the patch is applied we can see it is sorting improperly. After the second patch, we fail because of a missing dependency Installing the module liblibrary-callnumber-lc-perl in debian and running the tests We now pass all 10 The last patch adds the dependency. Patches pass all other QA tests, Created attachment 18780 [details] [review] bug 6281: add test case for sorting LC call numbers correctly My thanks to Michael Flanagan of UCAR for providing some of these examples. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Created attachment 18781 [details] [review] bug 6281: introduce LC::CallNumber::LC for sorting LC call numbers This replaces the previous hand-coded normalizer. Because LC::CallNumber::LC appears to reject strings that aren't valid LC call numbers, significant changes to the test cases were made as well -- however, the one that really counts is the last one which verifies the sorting. To recalculate the call number sort key for each item, it is necessary to run misc/maintenance/touch_all_items.pl To test, create item records with the following call numbers, setting the classification sort to 'lcc': QC100 .U57 NO. 555 1986 QC145 .A57 V.12 1980 QC145.45 .H4 D65 1998 QC995 .E29 1997 Next, make a report of them in the inventory tool. The items should be sorted in the above order. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Created attachment 18782 [details] [review] bug 6281: add Library::CallNumber::LC as a required Perl dependency This module is currently packaged by Debian for Wheezy and by Ubuntu for Precise and Quantal. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Pushed to master. This patch has been pushed to 3.12.x, will be in 3.12.2. Thanks Galen! Pushed to 3.10.x, will be in 3.10.8 Pushed to 3.8.x, will be in 3.8.15 |