Bug 6281 - Bug in LCC sort routine
Summary: Bug in LCC sort routine
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: PC Windows
: P5 - low normal (vote)
Assignee: Galen Charlton
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks: 9370
  Show dependency treegraph
 
Reported: 2011-04-28 13:39 UTC by flanagan
Modified: 2014-12-07 20:02 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
bug 6281: add test case for sorting LC call numbers correctly (1.88 KB, patch)
2013-01-09 02:16 UTC, Galen Charlton
Details | Diff | Splinter Review
bug 6281: introduce LC::CallNumber::LC for sorting LC call numbers (4.04 KB, patch)
2013-01-09 02:17 UTC, Galen Charlton
Details | Diff | Splinter Review
bug 6281: add Library::CallNumber::LC as a required Perl dependency (936 bytes, patch)
2013-01-09 02:17 UTC, Galen Charlton
Details | Diff | Splinter Review
bug 6281: add test case for sorting LC call numbers correctly (1.93 KB, patch)
2013-01-18 20:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
bug 6281: introduce LC::CallNumber::LC for sorting LC call numbers (4.10 KB, patch)
2013-01-18 20:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
bug 6281: add Library::CallNumber::LC as a required Perl dependency (991 bytes, patch)
2013-01-18 20:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
bug 6281: add test case for sorting LC call numbers correctly (1.93 KB, patch)
2013-05-30 15:12 UTC, Galen Charlton
Details | Diff | Splinter Review
bug 6281: introduce LC::CallNumber::LC for sorting LC call numbers (4.10 KB, patch)
2013-05-30 15:13 UTC, Galen Charlton
Details | Diff | Splinter Review
bug 6281: add Library::CallNumber::LC as a required Perl dependency (991 bytes, patch)
2013-05-30 15:13 UTC, Galen Charlton
Details | Diff | Splinter Review
bug 6281: add test case for sorting LC call numbers correctly (1.99 KB, patch)
2013-06-08 04:13 UTC, Chris Cormack
Details | Diff | Splinter Review
bug 6281: introduce LC::CallNumber::LC for sorting LC call numbers (4.15 KB, patch)
2013-06-08 04:13 UTC, Chris Cormack
Details | Diff | Splinter Review
bug 6281: add Library::CallNumber::LC as a required Perl dependency (1.01 KB, patch)
2013-06-08 04:13 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description flanagan 2011-04-28 13:39:12 UTC
While doing some work in the Inventory Tool, I notice that Koha is ordering things strangely.  It seems to be trying to order things, albeit unsuccessful, according to Dewey rules.  I have my DefaultClassificationSource set to Library of Congress.  I'm not sure what the problem is, but it's doing things like putting
 
 QC145.45 .H4 D65 1998 before QC145 .A57 V.12 1980
 and
 QC995 .E29 1997 before QC100 .U57 NO. 555 1986
 
And when I give it a range between, say, QC150 and QC 250, it gives me QC2 and QC 20 as well.

Galen: "Fortunately, it will be easy for me to fix, as there is another open-source LCC sort routine that we can use as a replacement, it just needs to be wired in."
Comment 1 Galen Charlton 2012-08-17 15:37:23 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.
Comment 2 Galen Charlton 2013-01-09 02:16:32 UTC Comment hidden (obsolete)
Comment 3 Galen Charlton 2013-01-09 02:17:14 UTC Comment hidden (obsolete)
Comment 4 Galen Charlton 2013-01-09 02:17:17 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2013-01-18 20:05:00 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2013-01-18 20:05:11 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2013-01-18 20:05:18 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2013-02-28 09:08:57 UTC
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 ?
Comment 9 Marcel de Rooy 2013-03-29 12:50:17 UTC
Changing status to Failed QA for the need of clarification (see previous comment).
Thanks for your attention.
Comment 10 Galen Charlton 2013-05-30 15:12:30 UTC Comment hidden (obsolete)
Comment 11 Galen Charlton 2013-05-30 15:13:03 UTC Comment hidden (obsolete)
Comment 12 Galen Charlton 2013-05-30 15:13:13 UTC Comment hidden (obsolete)
Comment 13 Galen Charlton 2013-05-30 15:18:41 UTC
(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.
Comment 14 Chris Cormack 2013-06-08 04:12:52 UTC
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,
Comment 15 Chris Cormack 2013-06-08 04:13:07 UTC
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>
Comment 16 Chris Cormack 2013-06-08 04:13:16 UTC
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>
Comment 17 Chris Cormack 2013-06-08 04:13:26 UTC
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>
Comment 18 Galen Charlton 2013-06-10 15:01:47 UTC
Pushed to master.
Comment 19 Tomás Cohen Arazi 2013-06-28 15:19:29 UTC
This patch has been pushed to 3.12.x, will be in 3.12.2.

Thanks Galen!
Comment 20 Bernardo Gonzalez Kriegel 2013-07-08 14:18:39 UTC
Pushed to 3.10.x, will be in 3.10.8
Comment 21 Chris Hall 2013-07-17 07:52:31 UTC
Pushed to 3.8.x, will be in 3.8.15