lazy import via `mintpy.cli` for more responsive CLI #828

avalentino · 2022-08-13T09:10:55Z

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 #823 (comment).

Reminders

Pass Codacy code review (green)
Pass Circle CI test (green)
Make sure that your code follows our style. Use the other functions/files as a basis.
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.

yunjunz · 2022-08-16T01:03:14Z

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).

+ expand the lazy imports to all scripts in command lines + cmd_line_parse() content arrangement: - parse - import - check - default values + main() content arrangement: - parse args - import - run

+ docs/installation.md: update the instruction to install mintpy via path setup + adjust to make path setup method working on cmd: - add `#!/usr/bin/env python3` to the top of all cli/[a-z]*.py scripts - run `chmod +x` to all `cli/[a-z]*.py` scripts - run `chmod -x` to all `mintpy/[a-z]*.py` scripts, except for two (add_attribute.py and multi_transect.py).

+ setup: - remove gmtsar as it's included in extra - remove gps, as both proj and dateutil are not used anymore - add notes for geoid and isce

yunjunz

Thank you @avalentino for the PR and @jhkennedy for the review! I really like the changes here. All the command line helps are now significantly speeded up, which improves the user experience a lot. Cheers!

Update: I will wait for another 12 hours to merge the PR, in case you have any further suggestions, since I modified the PR significantly.

avalentino · 2022-09-21T06:39:50Z

@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?

yunjunz · 2022-09-21T07:10:57Z

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?

avalentino · 2022-09-21T07:35:02Z

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.

SENSOR_NAMES is now duplicated in mintpy.cli.prep_gamma

avalentino · 2022-09-21T08:53:06Z

OK, @yunjunz
Commit done.

avalentino · 2022-09-22T05:39:27Z

Thanks @yunjunz

yunjunz · 2022-09-22T05:57:02Z

Thank you @avalentino for the PR!

+ workflow/__init__.py: fix the UnboundLocalError of 'mintpy' + smallbaselineApp: bring back the individual module import (as proposed in the lazy import PR insarlab#828), to replace the workflow import, for better speed and robustness.

+ `smallbaselineApp`: bring back the individual module import (as proposed in the lazy import PR #828), to replace the workflow import, for better speed and robustness. + `workflow/__init__.py`: fix the UnboundLocalError of 'mintpy' + `info`: support `--compact` output to `--date / --num` options + general Codacy style suggestions

avalentino force-pushed the feature/import-time branch 4 times, most recently from 1213221 to b664bf3 Compare August 13, 2022 15:43

avalentino requested a review from yunjunz August 13, 2022 15:57

avalentino force-pushed the feature/import-time branch from f3e53db to 86a63a2 Compare August 14, 2022 09:05

yunjunz requested review from taliboliver and jhkennedy and removed request for taliboliver August 14, 2022 17:54

avalentino force-pushed the feature/import-time branch 2 times, most recently from 063991f to c667d4b Compare August 20, 2022 08:14

yunjunz changed the title ~~Improve import time to make the CLI more responsive~~ lazy import via mintpy.cli sub-package for more responsive CLI Sep 7, 2022

yunjunz changed the title ~~lazy import via mintpy.cli sub-package for more responsive CLI~~ lazy import via mintpy.cli for more responsive CLI Sep 7, 2022

yunjunz force-pushed the feature/import-time branch from 14ad18a to 8ddc101 Compare September 18, 2022 17:18

avalentino and others added 15 commits September 18, 2022 10:38

Move workflow and standard processing CLI to mintpy.cli

16cecac

Move image operation CLI to mintpy.cli

d873c9f

Move noise reduction / error correction CLI to mintpy.cli

c27efa4

Move misc CLI to mintpy.cli

1c6a6d2

Move pre-processing CLI to mintpy.cli

0192bd5

Move I/O CLI to mintpy.cli

cf44281

Move visualization CLI to mintpy.cli

693cfeb

Optimize import time

667c075

Cleanup

f575720

Fix invocation of main functions

1f6eab8

Update optional requirements

56cfd7e

auto_path: put module import to the top of functions

75ccfba

check defaults & objects

d780e89

a-g: expand lazy import to all scripts in CLI

362ce46

+ expand the lazy imports to all scripts in command lines + cmd_line_parse() content arrangement: - parse - import - check - default values + main() content arrangement: - parse args - import - run

codacy

cb1b628

yunjunz added 13 commits September 20, 2022 17:03

more intuitive sub-func names for save_*.py

a98f254

put mintpy.cli.diff import right before its usage

74f7867

Update add_attribute.py

95e30b0

merge prep_snap into cli/prep_snap.py

1ba7796

codacy

e609d85

ref_point: rm the unnecessary run_ref*_point()

36671a0

bugfix in save_kmz_ts.py

d22fc6f

TimeSeriesAnalysis(): remove all unused return statement

8c10da0

Delete .smallbaselineApp.py.swp

bd50940

simplify subset.py

951d070

setup: sort + rm pysolid, tqdm, pyaps, pillow, mosec

36304f9

refactor insar_vs_gps to rm dateutil dep

7d3f204

+ setup: - remove gmtsar as it's included in extra - remove gps, as both proj and dateutil are not used anymore - add notes for geoid and isce

yunjunz approved these changes Sep 21, 2022

View reviewed changes

Avoid the import of mintpy.objects to speedup overall import time.

abed198

SENSOR_NAMES is now duplicated in mintpy.cli.prep_gamma

add comments on the duplicated global vars

3b3a088

yunjunz merged commit eec16ba into insarlab:main Sep 21, 2022

avalentino deleted the feature/import-time branch September 22, 2022 08:51

yunjunz mentioned this pull request Sep 23, 2022

smallbaselineApp: replace workflow import w/ individual import #854

Merged

3 tasks

yunjunz mentioned this pull request Oct 18, 2022

secular: refactor A.4 GNSS time-series plots nisar-solid/ATBD#41

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lazy import via `mintpy.cli` for more responsive CLI #828

lazy import via `mintpy.cli` for more responsive CLI #828

avalentino commented Aug 13, 2022 •

edited by yunjunz

Loading

yunjunz commented Aug 16, 2022 •

edited

Loading

yunjunz left a comment •

edited

Loading

avalentino commented Sep 21, 2022 •

edited

Loading

yunjunz commented Sep 21, 2022 •

edited

Loading

avalentino commented Sep 21, 2022 •

edited

Loading

avalentino commented Sep 21, 2022

avalentino commented Sep 22, 2022

yunjunz commented Sep 22, 2022

lazy import via mintpy.cli for more responsive CLI #828

lazy import via mintpy.cli for more responsive CLI #828

Conversation

avalentino commented Aug 13, 2022 • edited by yunjunz Loading

yunjunz commented Aug 16, 2022 • edited Loading

yunjunz left a comment • edited Loading

Choose a reason for hiding this comment

avalentino commented Sep 21, 2022 • edited Loading

yunjunz commented Sep 21, 2022 • edited Loading

avalentino commented Sep 21, 2022 • edited Loading

avalentino commented Sep 21, 2022

avalentino commented Sep 22, 2022

yunjunz commented Sep 22, 2022

lazy import via `mintpy.cli` for more responsive CLI #828

lazy import via `mintpy.cli` for more responsive CLI #828

avalentino commented Aug 13, 2022 •

edited by yunjunz

Loading

yunjunz commented Aug 16, 2022 •

edited

Loading

yunjunz left a comment •

edited

Loading

avalentino commented Sep 21, 2022 •

edited

Loading

yunjunz commented Sep 21, 2022 •

edited

Loading

avalentino commented Sep 21, 2022 •

edited

Loading