package import, installation, and data download policy - should we make a decision to install a package/make a local copy of data without the user asking?

[bact]

Background

Currently (version 1.7.x), some pythainlp modules will install a package/packages automatically if it's not yet installed.

For example:

try:
    import deepcut
except ImportError:
    from pythainlp.tools import install_package
    install_package("deepcut")
    import deepcut

The mechanism may convenient and suitable for use case inside a Jupyter notebook, but may not be ideal for other use cases' environment, like running in a system that's need more control over which package should be in presented in the system.

Proposal

Remove the automatic installation.

Just import the package and let it fails if the package doesn't currently exist in the system.

Rational

The difference here is transparency.

Running code in Jupyter notebook, the user can see the code and see what's currently happening on the cell's output window.

Running code on a server, for example, it is not that visible for the user.

Files/modules that will be affected

• pythainlp/g2p/init.py (Manage extra requires + merge g2p and romanization to one transliterate module #153)
• pythainlp/ner/init.py
• pythainlp/romanization/pyicu.py
• pythainlp/romanization/thai2rom.py
• pythainlp/ulmfit_sent.py
• pythainlp/tag/init.py
• pythainlp/tokenize/deepcut.py
• pythainlp/tokenize/pyicu.py
• pythainlp/tools/init.py
• pythainlp/ulmfit/utils.py
• pythainlp/word_vector/thai2vec.py

[wannaphong]

อันนี้น่าจะแก้ไขปัญหาเวลาติดตั้งโมดูลได้นะครับ จะได้ลบโค้ดติดตั้ง auto เองทิ้งครับ https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-extras-optional-features-with-their-own-dependencies

[wannaphong]

ถ้าย้ายโมดูลที่ต้องติดตั้งเพิ่มมาไว้ใน setup.py น่าจะได้ไหมครับ @bact

[bact]

น่าสนใจครับ
ตอนนี้มีทางเลือกแบบนี้ครับ

ระหว่างรัน

• ถ้าพบว่า ImportError ให้ติดตั้งให้อัตโนมัติ (ใช้ install_package) -- ปัจจุบันโค้ดส่วนใหญ่เป็นแบบนี้
• ถ้าพบว่า ImportError ไปใช้โค้ดส่วนอื่นที่เป็น fallback, ถ้าทำได้
• ถ้าพบว่า ImportError ปล่อยตาย

ตอนติดตั้ง

• ระบุลงใน requirements.txt (ซึ่งอาจมีได้หลายไฟล์ เช่น requirements-travis.txt ใช้สำหรับตอน build)
• ระบุลงใน setup.py
  • install_requires สำหรับแพคเกจที่ต้องใช้จริงๆ
  • extras_require สำหรับแพคเกจที่จำเป็นสำหรับบางฟีเจอร์

[wannaphong]

ถ้าพบว่า ImportError ไปใช้โค้ดส่วนอื่นที่เป็น fallback, ถ้าทำได้

ผมคิดว่าคนที่ใช้ในฝั่งผู้ใช้งานทางเว็บจากเซิฟเวอร์จะไม่รู้ว่าถูกเปลี่ยนไปใช้โค้ดส่วนอื่นที่เป็น fallback อาจจะทำให้เกิดการสับสนผลลัพธ์ต่างกันได้ครับ

[bact]

ผมลองเพิ่ม extras_require ตามนี้นะครับ

{
    "icu": ["pyicu"],
    "ml": ["fastai", "numpy", "sklearn_crfsuite", "torch"],
    "pos": ["artagger"],
    "tokenize": ["deepcut", "pyicu"],
    "transliterate": ["epitran", "pyicu"],
}

เวลาติดตั้ง ก็ใช้

pip install pythainlp[extra,extra,...]

เช่น

pip install pythainlp[icu,pos]

ก็จะติดตั้ง pyicu กับ artagger ให้ด้วย เพิ่มเติมจากใน requirements.txt

ตรงนี้จะช่วยให้ผู้ใช้ควบคุมการติดตั้ง package ได้ - ทดสอบใน #153 ใช้การได้อยู่ครับ

แต่เรื่อง data ที่ดาวน์โหลดอัตโนมัติ (เช่น thainer, thai2rom) ยังมีอยู่ครับ

[bact]

ปัญหานึงที่พบ เมื่อลองติดตั้งทุก package เพื่อที่จะ test ก็คือ ตัว CI ทั้ง Travis และ AppVeyor จะรัน job ไม่จบ เนื่องจาก job มันยาว ติดตั้งหลาย package จนเกินเวลาที่มันยอมให้ใช้

ตรงนี้ในการใช้งานจริงคงไม่เป็นปัญหา แต่จะเป็นปัญหาสำหรับการพัฒนา/ทำ test

[bact]

ผมคิดว่า ถ้าไม่คิดปัญหาเรื่องลิขสิทธิ์และสัญญาอนุญาต โมเดลอย่าง thainer, thai2rom, thai2vec
ควรจะติดมากับ package หลักเลย (เมื่อสั่งติดตั้งด้วย option ที่เหมาะสม) เพื่อจะได้ไม่ต้องดาวน์โหลดระหว่างรัน

ตรงนี้มีผลกับเวลาที่ใช้ในการ build เพื่อ test ในระบบ CI บน github ด้วย เพราะทุกครั้งที่ build ก็ต้องโหลดใหม่ทุกครั้ง เสียเวลาเพิ่มขึ้นมาก (ก่อนหน้านี้ไม่รู้สึกนัก เพราะ test coverage ยังไปไม่ถึงฟังก์ชันที่ใช้ข้อมูลเหล่านั้น พอตอนนี้ test ไปถึงแล้ว ทำให้แต่ละ build ใช้เวลาเป็นสิบนาที)

[bact]

ทาง @wannaphongcom มีความเห็นว่า โมเดลบางตัว 1) มีขนาดใหญ่ และ 2) มีการปรับปรุงต่อเนื่อง ทำให้อาจไม่สะดวกที่จะถูกติดตั้งมาแต่แรก

ประเด็น (1) น่าจะไม่มีปัญหา ถ้าเราให้ผู้ใช้เลือกได้ว่าจะติดตั้งหรือไม่

ประเด็น (2) อาจจะพอลดจำนวนการดาวน์โหลดได้ ถ้ามีการดาวน์โหลดข้อมูลมาตรฐาน (ไม่จำเป็นต้องเป็นรุ่นใหม่ล่าสุด) ในตอนติดตั้งแพคเกจ - ทำให้ตอน test ตอนพัฒนา ก็ยังใช้โมเดลตัวเก่า (ที่มากับแพคเกจ) รันโปรแกรมได้อยู่ (ส่วนใครจะไปดาวน์โหลดรุ่นใหม่ล่าสุดเองก็ทำเพิ่มเติมได้) -- ตรงนี้อาจใช้วิธีแก้ไข setup.py ให้มีการดาวน์โหลดข้อมูลที่จำเป็น 1 ครั้งตอนติดตั้ง

[bact]

• ตอนนี้เรื่อง import น่าจะถือว่าแก้ไขได้แล้ว ด้วยการใช้ extras_package (ดูรายละเอียดได้ใน PR Manage extra requires + merge g2p and romanization to one transliterate module #153) และมี workaround สำหรับ artagger บน Windows (artagger installation workaround on Windows test (AppVeyor) + add unit test #157)
• ส่วนเรื่องการดาวน์โหลดข้อมูล (โมเดล, รายการคำ, สถิติ, ฯลฯ) เป็นประเด็นที่ต้องหารือกันต่อไป

[bact]

Will close this as parts of the problem (on extra lib imports) are solved.

Will open new issue specifically on the external data loading.