bjlittle / geovista

Cartographic rendering and mesh analytics powered by PyVista
https://geovista.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
159 stars 26 forks source link

support lazy import #669

Closed bjlittle closed 8 months ago

bjlittle commented 9 months ago

πŸš€ Pull Request

Description

This pull-request addresses the slow import geovista time, which can take between ~2.5 - 3secs.

Incorporating these changes reduces this overhead to >100msecs e.g.,

> python -X importtime -c "import geovista"

import time: self [us] | cumulative | imported package
import time:       358 |        358 |   _io
import time:        45 |         45 |   marshal
import time:      1122 |       1122 |   posix
import time:       447 |       1971 | _frozen_importlib_external
import time:       142 |        142 |   time
import time:       143 |        285 | zipimport
import time:       343 |        343 |     _codecs
import time:       301 |        644 |   codecs
import time:      2160 |       2160 |   encodings.aliases
import time:      3720 |       6522 | encodings
import time:      1399 |       1399 | encodings.utf_8
import time:       157 |        157 | _signal
import time:        39 |         39 |     _abc
import time:       129 |        167 |   abc
import time:       185 |        351 | io
import time:        55 |         55 |       _stat
import time:        76 |        130 |     stat
import time:       983 |        983 |     _collections_abc
import time:        43 |         43 |       genericpath
import time:        77 |        120 |     posixpath
import time:       394 |       1625 |   os
import time:        75 |         75 |   _sitebuiltins
import time:      2670 |       2670 |   encodings.ascii
import time:      6912 |       6912 |   _distutils_hack
import time:      2233 |       2233 |   types
import time:      2212 |       2212 |       warnings
import time:      2422 |       4633 |     importlib
import time:      2316 |       2316 |     importlib._abc
import time:       448 |       7396 |   importlib.util
import time:       162 |        162 |   importlib.machinery
import time:      1843 |       1843 |   sitecustomize
import time:       296 |        296 |   usercustomize
import time:      8697 |      31905 | site
import time:      1591 |       1591 |   __future__
import time:       375 |        375 |             _operator
import time:      1864 |       2238 |           operator
import time:       495 |        495 |               itertools
import time:      1072 |       1072 |               keyword
import time:       941 |        941 |               reprlib
import time:       400 |        400 |               _collections
import time:      5082 |       7988 |             collections
import time:       135 |        135 |             _functools
import time:      2501 |      10624 |           functools
import time:      3972 |      16833 |         enum
import time:       201 |        201 |           _sre
import time:      2777 |       2777 |             re._constants
import time:      2517 |       5294 |           re._parser
import time:      1931 |       1931 |           re._casefix
import time:      2924 |      10348 |         re._compiler
import time:      1470 |       1470 |         copyreg
import time:      4198 |      32847 |       re
import time:      1530 |       1530 |       _ast
import time:      2050 |       2050 |       contextlib
import time:      6311 |      42737 |     ast
import time:       827 |        827 |           _opcode
import time:      1356 |       2182 |         opcode
import time:      2321 |       4503 |       dis
import time:      1688 |       1688 |       collections.abc
import time:      2540 |       2540 |           token
import time:        55 |         55 |           _tokenize
import time:      3857 |       6451 |         tokenize
import time:      1324 |       7774 |       linecache
import time:      7455 |      21418 |     inspect
import time:      2806 |      66960 |   lazy_loader
import time:     12285 |      12285 |   geovista._version
import time:      5287 |      86122 | geovista

Granted, this is just delaying or spreading the inevitable cost of importing the larger third-party packages e.g., pyvista, vtk, numpy, pyproj, but from a user perspective this is perhaps attractive.

Closes #647


codecov[bot] commented 9 months ago

Codecov Report

Attention: 25 lines in your changes are missing coverage. Please review.

Comparison is base (79385ff) 94.91% compared to head (247ee97) 94.49%.

Files Patch % Lines
src/geovista/search.py 50.00% 5 Missing :warning:
src/geovista/bridge.py 69.23% 4 Missing :warning:
src/geovista/common.py 69.23% 4 Missing :warning:
src/geovista/filters.py 77.77% 2 Missing :warning:
src/geovista/geodesic.py 66.66% 2 Missing :warning:
src/geovista/geometry.py 80.00% 2 Missing :warning:
src/geovista/pantry/textures.py 66.66% 2 Missing :warning:
src/geovista/core.py 75.00% 1 Missing :warning:
src/geovista/gridlines.py 75.00% 1 Missing :warning:
src/geovista/pantry/__init__.py 83.33% 1 Missing :warning:
... and 1 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #669 +/- ## ========================================== - Coverage 94.91% 94.49% -0.42% ========================================== Files 128 128 Lines 4757 4778 +21 ========================================== Hits 4515 4515 - Misses 242 263 +21 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bjlittle commented 9 months ago

@banesullivan @tkoyama010

I'd be really interested to know your thoughts on lazy_loader and whether it's appropriate to be adopted in pyvista ?

Also see SPEC 1 β€” Lazy Loading of Submodules and Functions.

tkoyama010 commented 9 months ago

I'd be really interested to know your thoughts on lazy_loader and whether it's appropriate to be adopted in pyvista ?

Also see SPEC 1 β€” Lazy Loading of Submodules and Functions.

I am fine with that. Let's discuss this at https://github.com/pyvista/pyvista/discussions/5512 !

bjlittle commented 8 months ago

Reference https://github.com/scientific-python/lazy_loader/issues/50

pp-mo commented 8 months ago

Granted, this is just delaying or spreading the inevitable cost of importing the larger third-party packages e.g., pyvista, vtk, numpy, pyproj, but from a user perspective this is perhaps attractive.

I think maybe this is missing a point. The prime benefit to the user is never having to worry about which sub-modules are loaded by default, and which must be explicitly imported. In principle, you should always import everything you use, so

import geovista as gv
import geovista.pantry.meshes
data = gv.pantry.meshes.fesom()

But in practice, the user would rather just have all of geovista plugged in with a single import. And IIUC this is now possible.

import geovista as gv
data = gv.pantry.meshes.fesom()