MintPy icon indicating copy to clipboard operation
MintPy copied to clipboard

Improve import time to make the CLI more responsive

Open avalentino opened this issue 2 years ago • 1 comments

Description of proposed changes

The CLI stuff is move to dedicated modules in the mintpy.cli sub-package. In this way it is possible to define the single-entry point argument parser without importing all the processing modules. Processing modules are imported only when necessary depending on the selected sub-command.

See also https://github.com/insarlab/MintPy/pull/823#issuecomment-1204559435.

Reminders

  • [x] Pass Codacy code review (green)
  • [x] Pass Circle CI test (green)
  • [x] Make sure that your code follows our style. Use the other functions/files as a basis.
  • [x] If modifying functionality, describe changes to function behavior and arguments in a comment below the function declaration.
  • [ ] If adding new functionality, add a detailed description to the documentation and/or an example.

avalentino avatar Aug 13 '22 09:08 avalentino

Thank you @avalentino for the useful and large PR! A quick test shows that the mintpy response time is significantly reduced.

[Updated] I am currently occupied with the preparation of the UNAVCO ISCE+ short course and NISAR workshop, will look at this PR more carefully after that (sorry for the delay).

yunjunz avatar Aug 16 '22 01:08 yunjunz

@yunjunz there is a small issue. In mintpy.cli.prep_gamma there is

from mintpy.objects.sensor import SENSOR_NAMES

that triggers the import of numpy and h5py in mintpy.objects.giant.

You chan check it using the following command:

python3 -X importtime -m mintpy -h

My original solution was to have a local copy of the SENSOR_NAMES list, that, I agree, is not nice. Another option could be to move SENSOR_NAMES to another module that can be imported without triggering the loading of heavy-waight modules like h5py and numpy. Having lazy imports in mintpy.objects.giant is probably not a good idea. How do you propose to proceed?

avalentino avatar Sep 21 '22 06:09 avalentino

Thank you for the importtime command, that's very helpful. The triggering of numpy import is because of the non-empty mintpy.objects.__init__.py file (this should be changed in the next major version), but modifying this now would require quite a lot of changes. How about moving sensor.py from mintpy.objects to mintpy.utils sub-module?

mintpy.utils.__init__.py file is empty, and the related changes (of importing sensor) would be only a few places.

Update: rethinking about this makes me realize that making a copy as you did before was a pretty good solution! Shall we revert back to that?

yunjunz avatar Sep 21 '22 07:09 yunjunz

Update: rethinking about this makes me realize that making a copy as you did before was a pretty good solution! Shall we revert back to that?

Probably it is the easiest solution. If you already plan a refactoring, then it should not be a big issue.

avalentino avatar Sep 21 '22 07:09 avalentino

OK, @yunjunz Commit done.

avalentino avatar Sep 21 '22 08:09 avalentino

Thanks @yunjunz

avalentino avatar Sep 22 '22 05:09 avalentino

Thank you @avalentino for the PR!

yunjunz avatar Sep 22 '22 05:09 yunjunz