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

Clean up delphi_utils #1973

Open
2 of 3 tasks
melange396 opened this issue Jun 7, 2024 · 5 comments · Fixed by #1984
Open
2 of 3 tasks

Clean up delphi_utils #1973

melange396 opened this issue Jun 7, 2024 · 5 comments · Fixed by #1984
Assignees
Labels

Comments

@melange396
Copy link
Contributor

melange396 commented Jun 7, 2024

delphi_utils is a big hodge-podge of a package with a lot of dependencies, thus it has a relatively large net footprint and takes a while to install. Lets see if we can lessen that.

  • Move packages in setup.py that are unnecessary for a downstream user but still used in test/lint/ci, like: darker, freezegun, mock, moto, pydocstyle, pylint, pytest-cov, pytest, requests-mock. They can be put into something like a "requirements.test.txt" file that gets installed via the CI and/or Makefile.
  • Remove from setup.py any unused packages, as gitpython appears to be.
  • The covidcast package has its own wacky imports, which includes a bunch of imaging/graphics and geospatial libraries. It is only used in the delphi_utils.validator module for pulling data from the API and can be replaced with the lighter-weight delphi_epidata client instead. Doing that is not quite as easy as moving requirements entries around, and it is covered in in replace the python covidcast client in validator #1972.
@melange396
Copy link
Contributor Author

We could even put some of the esoteric usage and/or larger installation dependencies behind package "extras".

@dshemetov
Copy link
Contributor

dshemetov commented Jun 26, 2024

If we combine this with migrating over to pyproject.toml, we could use the optional-dependencies interface to separate out the dev dependencies. See here for an example of migrating over to pyproject.toml. Then, we could change Makefile in _delphi_utils_python to do pip install -e .[dev] and all the rest of the indicators Makefiles can stay the same.

@aysim319 aysim319 self-assigned this Jun 27, 2024
@melange396
Copy link
Contributor Author

migrating to using pyproject.toml helps unlock this issue: #1839

@dshemetov
Copy link
Contributor

dshemetov commented Jul 25, 2024

Gonna reopen this issue. Also, gonna edit your OP @melange396 so the subtasks are clearer because #1984 addressed the first of the subtasks and we could probably tackle the second in the Jenkins-fix follow-up too. The third subtask can be closed when we finish up #1972.

@dshemetov
Copy link
Contributor

dshemetov commented Aug 6, 2024

Thinking about the unused package subtask, I found deptry and ran it on _delphi_utils and it reported this:

(env) ➜  _delphi_utils_python git:(1972-replace-covidcast) ✗ deptry .
Scanning 31 files...

data_proc/geomap/geo_data_proc.py:13:8: DEP003 'requests' imported but it is a transitive dependency
data_proc/geomap/source-file-sanity-check.ipynb:23:8: DEP003 'matplotlib' imported but it is a transitive dependency
delphi_utils/flash_eval/eval_day.py:6:1: DEP003 'scipy' imported but it is a transitive dependency
delphi_utils/validator/datafetcher.py:13:8: DEP003 'requests' imported but it is a transitive dependency
requirements.txt: DEP002 'scs' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'darker' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'freezegun' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'mock' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'moto' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'pydocstyle' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'pylint' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'pytest-cov' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'pytest' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'requests-mock' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'xlrd' defined as a dependency but not used in the codebase
Found 16 dependency issues.

For more information, see the documentation: https://deptry.com/

The following will be handled by #2014, as they get moved to dev.

requirements.txt: DEP002 'darker' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'freezegun' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'mock' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'moto' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'pydocstyle' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'pylint' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'pytest-cov' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'pytest' defined as a dependency but not used in the codebase
requirements.txt: DEP002 'requests-mock' defined as a dependency but not used in the codebase
  • xlrd is needed by Pandas (maybe we should write pandas[excel] so this dependence is explicit? EDIT: pandas==1.5.3 does not support that extra).
  • scs is explicitly version pinned for cvxpy because of upstream issues
  • seems like a good idea to explicitly list requests as a dependecy (done in follow up)
  • flash we can probably ignore until needed and geomap scripts we can ignore until needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants