Page MenuHomePhabricator
Create Task
Maniphest T85240

use external package for isbn.py
Closed, ResolvedPublic
Actions

Description

The ISBN validation data in isbn.py have not been updated since September 2007. The ISBN ranges are updated regularly, meaning the isbn.py script is currently wrong. Rather than keeping this data up to date, the algorithm in isbn.py should be replaced with an external package which provides the same functionality and is py3 compatible, packaged on pypi, and is regularly updated as the ISBN ranges are issued and modified.

There are several such packages on pypi. See
https://pypi.python.org/pypi?%3Aaction=search&term=isbn&submit=search

The most useful is
https://pypi.python.org/pypi/python-stdnum

Also up to date:
https://github.com/nekobcn/isbnid

https://pypi.python.org/pypi/isbnlib / https://pypi.python.org/pypi/isbntools

https://pypi.python.org/pypi/isbn_hyphenate

Ideally isbn is able to use whichever library is installed.

Details

SubjectRepoBranchLines +/-
External isbn packages supportpywikibot/coremaster+145 -34
Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 ResolvedmurfelT85240 use external package for isbn.py
 ResolvedjayvdbT89996 Add isbn package dependency
 ResolvedXqtT89998 cosmetic changes has no tests

Event Timeline

jayvdb created this task.Dec 23 2014, 7:22 PM
jayvdb raised the priority of this task from to Needs Triage.
jayvdb updated the task description. (Show Details)
jayvdb added a project: Pywikibot-Scripts.
jayvdb subscribed.
jayvdb added a project: Google-Code-in-2014.Dec 23 2014, 7:29 PM

This has been proposed as a Google-Code-in-2014 task: http://www.google-melange.com/gci/task/view/google/gci2014/5871428750540800

jayvdb added a comment.Jan 7 2015, 3:16 PM

As the ISBN data in this script is very outdated, I dont see the benefit in keeping it. However, removing the old code/data could become problematic , especially if you dont provide perfect backwards compatibility. So, dont remove it in your changeset, otherwise your changeset might be hit with a -2. If you leave the old code in the file, unmodified, it can still be accessed by other scripts which might depend on it via an import .

If the old code can be easily used as a fallback if no external libraries exist, that would be nice. (but optional if it isnt easy to do)

After your changeset adding support for external libraries, another changeset can then remove all the old code/data, and then it doesnt matter if someone hits it with a -2.

murfel claimed this task.Jan 9 2015, 11:23 AM
gerritbot added a project: Patch-For-Review.Jan 11 2015, 3:37 PM

Change 184225 had a related patch set uploaded (by Murfel):
External isbn packages support

https://gerrit.wikimedia.org/r/184225

Patch-For-Review

murfel added a comment.Jan 11 2015, 4:06 PM

Given that the script uses a library, it can say for sure whether an isbn is correct and add Template:Please_check_ISBN or Template:Listed Invalid ISBN if it's not, so it's going to be more efficient.

jayvdb moved this task from Backlog to pywikibot on the Google-Code-in-2014 board.Jan 15 2015, 1:16 AM
jayvdb added a comment.Jan 15 2015, 1:23 AM

IMO {{Listed Invalid ISBN}} should only ever be added by a human, and hopefully a bibliophile, as "listed" means it appeared in print on the physical item if it had a physical item, or nearest equivalent for born-digital items. fwiw, I created that template 8 years ago; how time flies. Interesting to see it now has 650+ uses on en.wp, and has been translated into 14 other languages. ;-)

gerritbot subscribed.Jan 17 2015, 9:26 PM

Change 184225 merged by jenkins-bot:
External isbn packages support

https://gerrit.wikimedia.org/r/184225

jayvdb added a comment.Jan 17 2015, 9:56 PM

The next step for this bug is to:

  1. add python-stdnum as a dependency in setup.py
  2. add tests for cosmetic_changes use of isbn
  3. determine what to do with the ISBN rule in fixes.py - i.e. is it useful
jayvdb mentioned this in T89993: remove isbn script dependency in cosmetic_changes.Feb 19 2015, 8:17 PM
jayvdb mentioned this in T89996: Add isbn package dependency.Feb 19 2015, 8:29 PM
jayvdb added a subtask: T89996: Add isbn package dependency.
jayvdb added a subtask: T89998: cosmetic changes has no tests.Feb 19 2015, 8:35 PM
murfel mentioned this in rPWBC8a87eaecd950: External isbn packages support.Mar 4 2015, 8:46 AM
jayvdb mentioned this in T97887: ISBN.py breaks while formatting.May 2 2015, 10:57 AM
In T85240#984192, @jayvdb wrote:

The next step for this bug is to:

  1. add python-stdnum as a dependency in setup.py

Done, as an optional dependency

  1. add tests for cosmetic_changes use of isbn

Done.

  1. determine what to do with the ISBN rule in fixes.py - i.e. is it useful

Outstanding.

jayvdb removed a project: Patch-For-Review.May 2 2015, 10:58 AM
jayvdb set Security to None.
jayvdb closed subtask T89996: Add isbn package dependency as Resolved.May 7 2015, 4:16 PM
jayvdb added a comment.May 8 2015, 3:13 AM

Review of ISBN cosmetic_changes vs fixes is good first task.

jayvdb added a project: good first task.May 8 2015, 3:13 AM
Xqt closed subtask T89998: cosmetic changes has no tests as Resolved.May 18 2015, 1:28 PM
jayvdb closed this task as Resolved.Jun 9 2015, 6:18 AM

fixes ISBN normalises the ISBN syntax, and cosmetic change ISBN validates the ISBN and reformats it some more. definitely some overlap, but not as bad as first thought. That overlap can be addressed in T76323.

XZise mentioned this in T109190: cosmetic_changes crashes with wrong ISBN.Aug 15 2015, 6:54 PM
jayvdb mentioned this in T132919: Stale isbn hyphenation data.Apr 18 2016, 2:55 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL