Bug 24674 - Uncertain years for publicationyear/copyrightdate -- corrected
Summary: Uncertain years for publicationyear/copyrightdate -- corrected
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Janusz Kaczmarek
QA Contact: Testopia
URL:
Keywords:
: 24869 (view as bug list)
Depends on: 11046
Blocks: 29412
  Show dependency treegraph
 
Reported: 2020-02-17 13:19 UTC by Janusz Kaczmarek
Modified: 2021-11-11 12:33 UTC (History)
8 users (show)

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


Attachments
Bug 24674: Uncertain years for publicationyear/copyrightdate -- corrected (1.14 KB, patch)
2020-02-17 13:23 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 24674: Uncertain years for publicationyear/copyrightdate -- corrected (1.25 KB, patch)
2020-02-18 10:58 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 24674: Adjust tests and add new ones to cover changes (1.71 KB, patch)
2020-02-18 13:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24674: Uncertain years for publicationyear/copyrightdate -- corrected (1.37 KB, patch)
2021-09-21 14:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24674: Adjust tests and add new ones to cover changes (2.10 KB, patch)
2021-09-21 14:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24674: (follow-up) Simpler regex (3.94 KB, patch)
2021-09-21 14:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24674: Uncertain years for publicationyear/copyrightdate -- corrected (1.38 KB, patch)
2021-10-24 12:28 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24674: Adjust tests and add new ones to cover changes (2.11 KB, patch)
2021-10-24 12:28 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24674: (follow-up) Simpler regex (3.96 KB, patch)
2021-10-24 12:28 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Janusz Kaczmarek 2020-02-17 13:19:29 UTC
According to n. 4.3.8 od ISBD Consolidated Edition of 2011, valid for date description are also string like [1960-], [17-].  Currently strings like these are not interpreted correctly by C4::Biblio::_adjust_pubyear.
Comment 1 Janusz Kaczmarek 2020-02-17 13:23:35 UTC
Created attachment 99088 [details] [review]
Bug 24674: Uncertain years for publicationyear/copyrightdate -- corrected

This is a correction to Bug 11046: YYY- is also correct according to ISBD: uncertain year, but certain decade (cf. n. 4.3.8 in ISBD Consolidated Edition of 2011).  (Question mark is not mandatory.)
Comment 2 Janusz Kaczmarek 2020-02-18 10:58:31 UTC
Created attachment 99173 [details] [review]
Bug 24674: Uncertain years for publicationyear/copyrightdate -- corrected

An amended version:

This is a correction to Bug 11046: YYY-, YY--, Y---, are also according to ISBD: uncertain year, but certain decade/century/millennium (cf. n. 4.3.8 in ISBD Consolidated Edition of 2011).
(Question mark is not mandatory.)

In addition, in Poland a form with only one dash is quite common, like YY- (== YY--).
Comment 3 Janusz Kaczmarek 2020-02-18 11:18:26 UTC
Test plan:

o  put a date like [198-] or [18--] or [17-] in 260 $c and save the record
o  check the error log -- you should see something like:
   Argument "[198-]." isn't numeric in int at /usr/share/koha/lib/C4/Biblio.pm line 2832
o  apply the patch
o  open and save the record again (ModBiblio will be executed)
o  check the error log -- it should have no errors now
Comment 4 Jonathan Druart 2020-02-18 13:14:50 UTC
Hi Janusz,

One test is failing with your patch:

t/db_dependent/Biblio/TransformMarcToKoha.t .. 1/3
    #   Failed test 'Missing question mark'
    #   at t/db_dependent/Biblio/TransformMarcToKoha.t line 107.
    #          got: '1980'
    #     expected: '198-'
    # Looks like you failed 1 test of 10.

#   Failed test 'Testing _adjust_pubyear'
#   at t/db_dependent/Biblio/TransformMarcToKoha.t line 109.
Comment 5 Janusz Kaczmarek 2020-02-18 13:38:22 UTC
Jonathan, thank you.

But the result is on purpose.  I do want that 198- ([198-] in particular, with square braces) will be transformed into 1980.  It is perfectly correct to have a date like this without question mark (cf. ISBD 2011 examples, p. 183-184, https://www.ifla.org/files/assets/cataloguing/isbd/isbd-cons_20110321.pdf).

Could we correct the tests?  (I do not know how to do this.)
Comment 6 Jonathan Druart 2020-02-18 13:50:29 UTC
Created attachment 99186 [details] [review]
Bug 24674: Adjust tests and add new ones to cover changes
Comment 7 Jonathan Druart 2020-02-18 13:54:21 UTC
I adjusted the failing tests and add some more. I do not know if we need to adjust the year only if surrounded by square brackets.

Letting someone else to signoff in order to get another point of view.
Comment 8 Katrin Fischer 2020-02-18 14:04:22 UTC
I am wondering if this could be confusing to users as we display the information from publicationyear/copyrightdate in quite a lot of places. We just recently looked into the logic here too:

1999- = 1999
1999-2009 = 1999
19xx (very common  here) = NULL

So the user will see the first "4 digits" as year when not looking at the detail page. 

While publicationyear is text in the database (used by UNIMARC), copyrightdate is a smallint(6) and can only store numeric values. I wonder if we want or are treating both fields the same currently and if we should be less restrictive for publicationyear?

I'd love to get some more eyes on this - from the standard I think you are right. It's just the issue of display that worries me a little.
Comment 9 Janusz Kaczmarek 2020-02-18 14:08:51 UTC
(In reply to Jonathan Druart from comment #7)
> I adjusted the failing tests and add some more. I do not know if we need to
> adjust the year only if surrounded by square brackets.
> 
> Letting someone else to signoff in order to get another point of view.

I think we do not have to bother about the braces here.  In very formal ISBD a date like this should be surrounded by braces, but in older records the whole zone 4 could be in braces, not the individual elements (like: [Roma : s.n., 18--]).  The function _adjust_pubyear works well in both cases, paying attention on numbers only and characters after.
Comment 10 Janusz Kaczmarek 2020-02-18 14:15:07 UTC
(In reply to Katrin Fischer from comment #8)
> I am wondering if this could be confusing to users as we display the
> information from publicationyear/copyrightdate in quite a lot of places. We
> just recently looked into the logic here too:
> 
> 1999- = 1999
> 1999-2009 = 1999
> 19xx (very common  here) = NULL
> 
> So the user will see the first "4 digits" as year when not looking at the
> detail page. 
> 
> While publicationyear is text in the database (used by UNIMARC),
> copyrightdate is a smallint(6) and can only store numeric values. I wonder
> if we want or are treating both fields the same currently and if we should
> be less restrictive for publicationyear?
> 
> I'd love to get some more eyes on this - from the standard I think you are
> right. It's just the issue of display that worries me a little.

I get the point -- this is an important design question.  I totally agree, this effect can be and is misleading for the users.

But here I just wanted to get rid of the annoying errors in errorlog which are caused by the current version of the _adjust_pubyear function. (If it transforms [198-?] into 1980, it should do the same with [198-].
Comment 11 Janusz Kaczmarek 2020-02-18 15:11:55 UTC
But guess what will be displayed in this real life example:

Warszawa : Universitas, [19]89-.

http://katalog.nukat.edu.pl/lib/item?id=chamo:22190&fromLocationLink=false&theme=nukat

BTW, I'm wondering why, in standard installation od Koha, is the same ISBD element "Date of publication, production and/or distribution" [4.3] mapped to biblio.copyrightdate when coming from MARC 21 (= 260 $c), but to biblioitems.publicationyear when coming from UNIMARC...

Any clue?
Comment 12 Katrin Fischer 2020-02-18 15:54:13 UTC
(In reply to Janusz Kaczmarek from comment #11)
> But guess what will be displayed in this real life example:
> 
> Warszawa : Universitas, [19]89-.
> 
> http://katalog.nukat.edu.pl/lib/item?id=chamo:
> 22190&fromLocationLink=false&theme=nukat
> 
> BTW, I'm wondering why, in standard installation od Koha, is the same ISBD
> element "Date of publication, production and/or distribution" [4.3] mapped
> to biblio.copyrightdate when coming from MARC 21 (= 260 $c), but to
> biblioitems.publicationyear when coming from UNIMARC...
> 
> Any clue?

I have no idea to the why... my guess is it was an early on oversight that turned out really hard to fix later on. So now we have a lot of 'glue code' that makes both work right in a lot of places :(
Comment 13 Katrin Fischer 2020-07-24 12:35:04 UTC
*** Bug 24869 has been marked as a duplicate of this bug. ***
Comment 14 Fridolin Somers 2020-07-30 16:00:27 UTC
Should we also remove in C4::Biblio::_koha_modify_biblio :
$sth->execute(
...
$biblio->{'copyrightdate'} ? int($biblio->{'copyrightdate'}) : undef,

It generates a lot of warn in logs.
Comment 15 Marcel de Rooy 2021-09-20 14:09:21 UTC
From bug 11046:

This part of the regex made me wonder:
             (?<year>\d)[-]?[.Xx?]{3}
What does the minus do here after the first digit? Do we allow 1-234 or so?
Looking at the above, this must have been a typo (copy/paste) after all..
Comment 16 Marcel de Rooy 2021-09-20 14:11:44 UTC
Working a bit on this one
Comment 17 Marcel de Rooy 2021-09-20 14:19:48 UTC
(In reply to Katrin Fischer from comment #8)

> 1999- = 1999
> 1999-2009 = 1999
> 19xx (very common  here) = NULL

This observation seems to be incorrect. The code does this:

19xx = 1900
19--? = 1900
19-- = 19

The latter one is obviously wrong. (The suffix question mark should not be mandatory in the regex.)

A problem that I could see with the Polish way of Janusz here is that 17- should become 1700. But what if you would describe a year between 300 and 400?
3- or even 3-- would mean 3000 ?
Comment 18 Marcel de Rooy 2021-09-20 14:25:50 UTC
(In reply to Janusz Kaczmarek from comment #2)

> In addition, in Poland a form with only one dash is quite common, like YY-
> (== YY--).

With reference to former comment, this form is not in the ISBD document you refer to in the description btw.
Comment 19 Marcel de Rooy 2021-09-21 14:41:49 UTC
Created attachment 125108 [details] [review]
Bug 24674: Uncertain years for publicationyear/copyrightdate -- corrected

This is a correction to Bug 11046: YYY-, YY--, Y---, are also
correct according to ISBD: uncertain year, but certain decade/century
(cf. n. 4.3.8 in ISBD Consolidated Edition of 2011).
(Question mark is not mandatory.)

In addition, in Poland a form with one only dash is quite common,
like YY- (== YY--).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
See last follow-up.
Comment 20 Marcel de Rooy 2021-09-21 14:41:52 UTC
Created attachment 125109 [details] [review]
Bug 24674: Adjust tests and add new ones to cover changes

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased 21-09-21.
Comment 21 Marcel de Rooy 2021-09-21 14:41:57 UTC
Created attachment 125110 [details] [review]
Bug 24674: (follow-up) Simpler regex

The 'Polish notation' actually simplifies things.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 22 Marcel de Rooy 2021-09-22 13:40:10 UTC
You might be surprised what you find if you look for copyrightdate>0 and copyrightdate<1000 in your local catalogue..

You could use this in a where statement with touch_all_biblios
Comment 23 Katrin Fischer 2021-10-24 12:28:28 UTC
Created attachment 126810 [details] [review]
Bug 24674: Uncertain years for publicationyear/copyrightdate -- corrected

This is a correction to Bug 11046: YYY-, YY--, Y---, are also
correct according to ISBD: uncertain year, but certain decade/century
(cf. n. 4.3.8 in ISBD Consolidated Edition of 2011).
(Question mark is not mandatory.)

In addition, in Poland a form with one only dash is quite common,
like YY- (== YY--).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
See last follow-up.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Katrin Fischer 2021-10-24 12:28:32 UTC
Created attachment 126811 [details] [review]
Bug 24674: Adjust tests and add new ones to cover changes

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased 21-09-21.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2021-10-24 12:28:37 UTC
Created attachment 126812 [details] [review]
Bug 24674: (follow-up) Simpler regex

The 'Polish notation' actually simplifies things.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Jonathan Druart 2021-11-03 12:40:42 UTC
Is that correct to assume that nothing has been published before year 1000?
Comment 27 Marcel de Rooy 2021-11-03 14:05:12 UTC
(In reply to Jonathan Druart from comment #26)
> Is that correct to assume that nothing has been published before year 1000?

Certainly not :) But thats really an negligible edge case here. The advantage of defaulting to 4 positions for the large majority is greater. And still 300? works.

Another edge case is years B.C. with -300 or so.

Dont think we should focus on rare exceptions.
Comment 28 Jonathan Druart 2021-11-03 14:14:45 UTC
(In reply to Marcel de Rooy from comment #27)
> (In reply to Jonathan Druart from comment #26)
> > Is that correct to assume that nothing has been published before year 1000?
> 
> Certainly not :) But thats really an negligible edge case here. The
> advantage of defaulting to 4 positions for the large majority is greater.
> And still 300? works.
> 
> Another edge case is years B.C. with -300 or so.
> 
> Dont think we should focus on rare exceptions.

Maybe entire libraries are using mostly those exceptions :)
Comment 29 Marcel de Rooy 2021-11-03 14:37:32 UTC
Try this:

use Modern::Perl;
use Data::Dumper qw/Dumper/;
use C4::Biblio;
print C4::Biblio::_adjust_pubyear('2xx');

19.11 => 2000
master (without this patch) => Use of uninitialized value in print at zz line 5.
(Changes between 19,11 and master: now returns undef)
Both cases are not what we expected.

This passed QA. I would recommend to push it. Further fixes may follow.
Comment 30 Katrin Fischer 2021-11-03 22:27:35 UTC
I've been bitten by Koha assuming years are after 1000 for the date range search in the past - might be nice to tackle separately. When dealing with rare items and manuscripts < 1000 is totally realistic.
Comment 31 Marcel de Rooy 2021-11-04 08:34:56 UTC
(In reply to Katrin Fischer from comment #30)
> I've been bitten by Koha assuming years are after 1000 for the date range
> search in the past - might be nice to tackle separately. When dealing with
> rare items and manuscripts < 1000 is totally realistic.

I'm willing to work on it. See bug 29412.
Comment 32 Jonathan Druart 2021-11-05 11:14:36 UTC
Pushed to master for 21.11, thanks to everybody involved!