lazy import via `mintpy.cli` for more responsive CLI by avalentino · Pull Request #828 · insarlab/MintPy

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 jhkennedy and taliboliver 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
yunjunz merged 64 commits intoinsarlab:mainfrom
avalentino:feature/import-time

avalentino commented Aug 13, 2022 •

edited by yunjunz

Loading

Uh oh!

yunjunz commented Aug 16, 2022 •

edited

Loading

Uh oh!

yunjunz left a comment •

edited

Loading

Uh oh!

avalentino commented Sep 21, 2022 •

edited

Loading

Uh oh!

yunjunz commented Sep 21, 2022 •

edited

Loading

Uh oh!

avalentino commented Sep 21, 2022 •

edited

Loading

Uh oh!

avalentino commented Sep 21, 2022

Uh oh!

avalentino commented Sep 22, 2022

Uh oh!

yunjunz commented Sep 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

avalentino commented Aug 13, 2022 • edited by yunjunz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yunjunz commented Aug 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yunjunz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avalentino commented Sep 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yunjunz commented Sep 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avalentino commented Sep 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avalentino commented Sep 21, 2022

Uh oh!

avalentino commented Sep 22, 2022

Uh oh!

yunjunz commented Sep 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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