Skip to content

Conversation

@ice-tong
Copy link
Collaborator

@ice-tong ice-tong commented Jun 15, 2022

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

Refactor mim install for better user experience. Mainly include the following enhancements:

  • You can use mim install in the same way you use pip install!
  • Download and install the OpenMMLab packages from PyPI instead of downloading from Github.
  • Automatically solve and install OpenMMLab-related dependencies via pip resolver.

Modification

1. commands/install.py

  • Receive all arguments and options to pip for processing, which means you can use mim install just like pip install.
  • Use pip._internal.commands.create_command to call pip for installation instead of subprocess.check_call . This allows us to hack the pip code.
  • Instead of downloading the source code from Github and then building and installing it, download and install OpenMMLab packages from PyPI like pip install.
  • Hack the Distribution in pip makes mim extra requirements added when returning dependencies. If mim extra requirements do not exist, download the source distribution package of the corresponding version from PyPI, and extract and parse mminstall.txt as mim extra requirements.
  • Since the new version of the OpenMMLab packages already supports packaging mim resource files into the distribution package, mim install no longer manually copies the required files to the .mim directory.

2. tests/test_*.py

  • It is not supported to give a Github link as a find link, and install the corresponding package of the Github link, delete the tests for the corresponding function in tests/test_install.py.
  • Since mminstall.txt has not been included in the previously released mmcls package, mmcv-full cannot be installed automatically, so the steps to install mmcv-full have been added to several test files.
  • Replaced a newer version of mmcls in tests/test_list.py since mim install no longer creates the .mim folder.

BC-breaking (Optional)

  • Specifying Github project link as find link is not supported, you can use mim install git+<vcs project url> instead.
  • mim install will not create and copy the required resource files to the .mim folder for the OpenMMLab packages, because newer versions of OpenMMLab packages already package the .mim into the distribution, please use these newer versions.

Use cases (Optional)

Already manually tested locally

  • mim install mmdet mmcls
  • mim install -r requirements.txt
  • mim install git+https://github.com/open-mmlab/mmdetection.git
  • mim install -e <path>
  • mim install mmdet -i <url> -f <url>

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit tests to ensure the correctness.
  3. If the modification has potential influence on downstream projects, this PR should be tested with downstream projects, like MMDet or MMCls.
  4. The documentation has been modified accordingly, like docstring or example tutorials.

@ice-tong ice-tong changed the title [Refactor] refactor mim install [WIP][Refactor] refactor mim install Jun 15, 2022
@codecov
Copy link

codecov bot commented Jun 15, 2022

Codecov Report

Merging #132 (2a16e75) into main (7421125) will increase coverage by 20.81%.
The diff coverage is 68.08%.

❗ Current head 2a16e75 differs from pull request most recent head a08dd99. Consider uploading reports for the commit a08dd99 to get more accurate results

@@             Coverage Diff             @@
##             main     #132       +/-   ##
===========================================
+ Coverage   32.77%   53.59%   +20.81%     
===========================================
  Files          22       22               
  Lines        1620     1532       -88     
  Branches      371      328       -43     
===========================================
+ Hits          531      821      +290     
+ Misses       1045      585      -460     
- Partials       44      126       +82     
Flag Coverage Δ
unittests 53.59% <68.08%> (+20.81%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mim/commands/install.py 69.59% <68.08%> (+0.95%) ⬆️
mim/click/customcommand.py 27.27% <0.00%> (+9.09%) ⬆️
mim/commands/train.py 37.07% <0.00%> (+12.35%) ⬆️
mim/utils/utils.py 67.88% <0.00%> (+17.07%) ⬆️
mim/commands/test.py 43.56% <0.00%> (+20.79%) ⬆️
mim/commands/run.py 42.46% <0.00%> (+23.28%) ⬆️
mim/click/option.py 54.05% <0.00%> (+24.32%) ⬆️
mim/commands/gridsearch.py 45.93% <0.00%> (+30.23%) ⬆️
mim/commands/search.py 54.00% <0.00%> (+41.24%) ⬆️
mim/commands/list.py 77.27% <0.00%> (+52.27%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7421125...a08dd99. Read the comment docs.

@ice-tong ice-tong requested a review from zhouzaida June 17, 2022 06:21
@ice-tong ice-tong marked this pull request as ready for review June 21, 2022 01:54
@zhouzaida zhouzaida requested a review from ZwwWayne June 21, 2022 03:20
@ice-tong ice-tong force-pushed the yancong-refactor-install branch from b72475a to 7c98032 Compare June 21, 2022 04:40
@ice-tong ice-tong changed the title [WIP][Refactor] refactor mim install [Refactor] refactor mim install Jun 21, 2022
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.

2 participants