Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BugFix] Update code to be compatible with py38 #48

Merged
merged 1 commit into from
May 6, 2024

Conversation

rahul-tuli
Copy link
Member

@rahul-tuli rahul-tuli commented May 6, 2024

In one of the places we use shorthand operator |= to combine two dicts, this led to a py38 test failure as this syntax was introduced in python 3.9; Fix is to use dict.update(...) over |=

Use dict.update(...) over |= as it was added in py39

Test plan: re-ran failing test with py38 and it works:

(.venv) ➜  sparseml git:(main) pytest tests/sparseml/transformers/sparsification/test_compress_tensor_utils.py::test_sparse_model_reload
======================================================================= test session starts ========================================================================
rootdir: /root/projects/sparseml
configfile: pyproject.toml
plugins: mock-3.14.0, hydra-core-1.3.2, rerunfailures-14.0
collected 5 items                                                                                                                                                  

tests/sparseml/transformers/sparsification/test_compress_tensor_utils.py .....                                                                               [100%]

========================================================================= warnings summary =========================================================================

@dbogunowicz dbogunowicz merged commit a21348d into main May 6, 2024
2 checks passed
@dbogunowicz dbogunowicz deleted the fix-syntax-for-py38 branch May 6, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants