chore(*): change installation of spacy weights to runtime #462

KrishPatel13 · 2023-08-10T17:36:53Z

What kind of change does this PR introduce?

feature

Summary
Motivation: #461

Checklist

My code follows the style guidelines of OpenAdapt
I have performed a self-review of my code
If applicable, I have added tests to prove my fix is functional/effective
I have linted my code locally prior to submission
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation (e.g. README.md, requirements.txt)
New and existing unit tests pass locally with my changes

How can your code be run and tested?

pip uninstall `en_core_web_trf`
# Follow Setup Instructions in README.md
pytest -s

Other information

manual steup instruciton in README.md

installion from both the install scripts

KrishPatel13 · 2023-08-10T17:44:30Z

This is how pytest will look like when we do not have the required spacy weight model installed:

(openadapt-py3.10) PS P:\OpenAdapt AI - MLDS AI\cloned_repo\test_other\OpenAdapt> pytest
============================================================================== test session starts ==============================================================================
platform win32 -- Python 3.10.11, pytest-7.1.3, pluggy-1.2.0
rootdir: P:\OpenAdapt AI - MLDS AI\cloned_repo\test_other\OpenAdapt
plugins: anyio-3.7.1, Faker-19.2.0, cov-2.10.0
collected 10 items / 1 error

==================================================================================== ERRORS =====================================================================================
________________________________________________________________ ERROR collecting tests/openadapt/test_scrub.py _________________________________________________________________
tests\openadapt\test_scrub.py:9: in <module>
    from openadapt import config, scrub
openadapt\scrub.py:25: in <module>
    NLP_ENGINE_TRF = SCRUB_PROVIDER_TRF.create_engine()
C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\presidio_analyzer\nlp_engine\nlp_engine_provider.py:91: in create_engine
    engine = nlp_engine_class(nlp_engine_opts)
C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\presidio_analyzer\nlp_engine\spacy_nlp_engine.py:36: in __init__        
    self.nlp = {
C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\presidio_analyzer\nlp_engine\spacy_nlp_engine.py:37: in <dictcomp>      
    lang_code: spacy.load(model_name, disable=["parser"])
C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\spacy\__init__.py:51: in load
    return util.load_model(
C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\spacy\util.py:472: in load_model
    raise IOError(Errors.E050.format(name=name))
E   OSError: [E050] Can't find model 'en_core_web_trf'. It doesn't seem to be a Python package or a valid path to a data directory.
=============================================================================== warnings summary ================================================================================ 
C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pycountry\__init__.py:10
  C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pycountry\__init__.py:10: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
    import pkg_resources

C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pkg_resources\__init__.py:2871
  C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pkg_resources\__init__.py:2871: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('mpl_toolkits')`.
  Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
    declare_namespace(pkg)

C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pkg_resources\__init__.py:2871
  C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pkg_resources\__init__.py:2871: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('sphinxcontrib')`.
  Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
    declare_namespace(pkg)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================================================ short test summary info ============================================================================ 
ERROR tests/openadapt/test_scrub.py - OSError: [E050] Can't find model 'en_core_web_trf'. It doesn't seem to be a Python package or a valid path to a data directory.
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! 
========================================================================= 3 warnings, 1 error in 6.29s ========================================================================== 
(openadapt-py3.10) PS P:\OpenAdapt AI - MLDS AI\cloned_repo\test_other\OpenAdapt>

KrishPatel13 · 2023-08-10T17:59:24Z

@abrichr Ready for review!

KrishPatel13 · 2023-08-10T18:04:46Z

Some Screenshots:

Mustaballer

If I do pip install openadapt, will this also download the spacy weights?
Also I don't think the PR title should be a feat. I think chore might be better since this is more like an internal process adjustment or a change related to the build process rather than introducing a new user-facing feature. What do you think?

KrishPatel13 · 2023-08-10T18:11:03Z

Now spacy weight will be downloaded in runtime. So whenever the user had to use the scrub code either via record or pytest or visualization, then it will check for the installation for the spacy weight and if not found it will install and then it will continue. This will be pure runtime and no need of adding this to installtion scripts or manual setup or toml or poetry ;-)

Mustaballer

lgtm!

abrichr · 2023-08-10T19:40:48Z

Thanks @KrishPatel13 !

What do you think about the approach mentioned here explosion/spaCy#4592 (comment) :

import spacy

spacy_model_name = 'de_core_news_sm'
if not spacy.util.is_package(spacy_model_name):
    spacy.cli.download(spacy_model_name)

Also, what do you think about skipping tests which depend on spacy if the model is not installed with @pytest.mark.skipif ?

KrishPatel13 · 2023-08-10T19:42:55Z

Good point, I will include that.

OpenAdaptAI#462 (comment)

KrishPatel13 · 2023-08-10T19:56:44Z

We will ahve to skip all of the test functions in test_scrub if the required spacy is not installed. Hence,

A better approach: https://docs.pytest.org/en/7.1.x/how-to/skipping.html#:~:text=Skip%20all%20test%20functions%20of%20a%20class%20or%20module&text=If%20you%20want%20to%20skip,skipif(...)

address comment: OpenAdaptAI#462 (comment)

spacy miodel is ont installed

KrishPatel13 · 2023-08-10T20:42:16Z

How pytest will be working if spacy model is not found

(openadapt-py3.10) PS P:\OpenAdapt AI - MLDS AI\cloned_repo\test_other\OpenAdapt> pip uninstall en_core_web_trf
WARNING: Skipping en_core_web_trf as it is not installed.
(openadapt-py3.10) PS P:\OpenAdapt AI - MLDS AI\cloned_repo\test_other\OpenAdapt> pytest
================================ test session starts ================================
platform win32 -- Python 3.10.11, pytest-7.1.3, pluggy-1.2.0
rootdir: P:\OpenAdapt AI - MLDS AI\cloned_repo\test_other\OpenAdapt
plugins: anyio-3.7.1, Faker-19.2.0, cov-2.10.0
collected 25 items

tests\openadapt\test_crop.py .                                                 [  4%]
tests\openadapt\test_events.py .......                                         [ 32%]
tests\openadapt\test_scrub.py sssssssssssssss                                  [ 92%]
tests\openadapt\test_summary.py ..                                             [100%]

================================= warnings summary ==================================
C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pycountry\__init__.py:10
  C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pycountry\__init__.py:10: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html      
    import pkg_resources

C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pkg_resources\__init__.py:2871
  C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pkg_resources\__init__.py:2871: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('mpl_toolkits')`.
  Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
    declare_namespace(pkg)

C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pkg_resources\__init__.py:2871
  C:\Users\Krish Patel\AppData\Local\pypoetry\Cache\virtualenvs\openadapt-DSRh12US-py3.10\lib\site-packages\pkg_resources\__init__.py:2871: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('sphinxcontrib')`.
  Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
    declare_namespace(pkg)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
==================== 10 passed, 15 skipped, 3 warnings in 8.55s ===================== 
(openadapt-py3.10) PS P:\OpenAdapt AI - MLDS AI\cloned_repo\test_other\OpenAdapt> pip show en_core_web_trf
WARNING: Package(s) not found: en_core_web_trf
(openadapt-py3.10) PS P:\OpenAdapt AI - MLDS AI\cloned_repo\test_other\OpenAdapt> imestamp=

KrishPatel13 · 2023-08-10T20:43:48Z

@abrichr Ready for merging ;-)

addressed your comment

abrichr · 2023-08-11T18:39:41Z

tests/openadapt/test_scrub.py

+from openadapt import config
+
+if not spacy.util.is_package(config.SPACY_MODEL_NAME):
+    pytestmark = pytest.mark.skip(reason="SpaCy model not installed.")


This does not appear to be used anywhere, is that intentional?

It is used when running pytest.

Basically, when the user does not have the Spacy Model installed then all of the tests in the file will be skipped.

If the model is installed then, it will import scrub and continue to run the tests

…I#462) * remove spacy from manual steup instruciton in README.md * remove spacy installion from both the install scripts * add todo * test runtime code for spacy installtion * pyetst passes even if spacy model is not installed * addressing: OpenAdaptAI#462 (comment) * add spacy-trnasformers address comment: OpenAdaptAI#462 (comment) * skip all the tests in test_scrub if spacy miodel is ont installed * format

Krish Patel added 2 commits August 10, 2023 13:34

remove spacy from

29c5200

manual steup instruciton in README.md

remove spacy

9ae9f20

installion from both the install scripts

KrishPatel13 self-assigned this Aug 10, 2023

KrishPatel13 marked this pull request as draft August 10, 2023 17:37

KrishPatel13 changed the title ~~feat: install spacy in runtime~~ feat(*): change installation of spacy weights to runtime Aug 10, 2023

KrishPatel13 mentioned this pull request Aug 10, 2023

Install spacy weights at runtime #461

Closed

add todo

cbc8136

Krish Patel added 2 commits August 10, 2023 13:53

test runtime code for spacy installtion

246ac69

pyetst passes even if spacy model is not installed

cb5ce6e

KrishPatel13 marked this pull request as ready for review August 10, 2023 18:00

KrishPatel13 requested review from abrichr and Mustaballer August 10, 2023 18:01

Mustaballer reviewed Aug 10, 2023

View reviewed changes

KrishPatel13 changed the title ~~feat(*): change installation of spacy weights to runtime~~ chore(*): change installation of spacy weights to runtime Aug 10, 2023

KrishPatel13 requested a review from Mustaballer August 10, 2023 18:16

Mustaballer approved these changes Aug 10, 2023

View reviewed changes

Merge branch 'OpenAdaptAI:main' into feat/install_spacy_runtime

cb7b1b0

addressing:

af0e794

OpenAdaptAI#462 (comment)

Krish Patel added 2 commits August 10, 2023 16:04

add spacy-trnasformers

2e47d5a

address comment: OpenAdaptAI#462 (comment)

skip all the tests in test_scrub if

6bb62c0

spacy miodel is ont installed

format

1ad479a

abrichr reviewed Aug 11, 2023

View reviewed changes

abrichr merged commit 479937e into OpenAdaptAI:main Aug 17, 2023

abrichr deleted the feat/install_spacy_runtime branch August 17, 2023 20:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(*): change installation of spacy weights to runtime #462

chore(*): change installation of spacy weights to runtime #462

KrishPatel13 commented Aug 10, 2023 •

edited

Loading

KrishPatel13 commented Aug 10, 2023

KrishPatel13 commented Aug 10, 2023

KrishPatel13 commented Aug 10, 2023

Mustaballer left a comment

KrishPatel13 commented Aug 10, 2023 •

edited

Loading

Mustaballer left a comment

abrichr commented Aug 10, 2023

KrishPatel13 commented Aug 10, 2023

KrishPatel13 commented Aug 10, 2023

KrishPatel13 commented Aug 10, 2023 •

edited

Loading

KrishPatel13 commented Aug 10, 2023

abrichr Aug 11, 2023

KrishPatel13 Aug 11, 2023

KrishPatel13 Aug 11, 2023

chore(*): change installation of spacy weights to runtime #462

chore(*): change installation of spacy weights to runtime #462

Conversation

KrishPatel13 commented Aug 10, 2023 • edited Loading

KrishPatel13 commented Aug 10, 2023

KrishPatel13 commented Aug 10, 2023

KrishPatel13 commented Aug 10, 2023

Mustaballer left a comment

Choose a reason for hiding this comment

KrishPatel13 commented Aug 10, 2023 • edited Loading

Mustaballer left a comment

Choose a reason for hiding this comment

abrichr commented Aug 10, 2023

KrishPatel13 commented Aug 10, 2023

KrishPatel13 commented Aug 10, 2023

KrishPatel13 commented Aug 10, 2023 • edited Loading

KrishPatel13 commented Aug 10, 2023

abrichr Aug 11, 2023

Choose a reason for hiding this comment

KrishPatel13 Aug 11, 2023

Choose a reason for hiding this comment

KrishPatel13 Aug 11, 2023

Choose a reason for hiding this comment

KrishPatel13 commented Aug 10, 2023 •

edited

Loading

KrishPatel13 commented Aug 10, 2023 •

edited

Loading

KrishPatel13 commented Aug 10, 2023 •

edited

Loading