Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Level3File import is a bit bloated #1901

Closed
dlaw opened this issue Jun 8, 2021 · 1 comment · Fixed by #1928
Closed

Level3File import is a bit bloated #1901

dlaw opened this issue Jun 8, 2021 · 1 comment · Fixed by #1928
Labels
Type: Bug Something is not working like it should
Milestone

Comments

@dlaw
Copy link
Contributor

dlaw commented Jun 8, 2021

I am using the Level3File class to process some radar data on a server. I do not use any other features from MetPy. I have two issues with the import proess:

  1. Upon importing Level3File, I always receive an error Cannot import USCOUNTIES and USSTATES without Cartopy installed. I intentionally do not have cartopy installed, because I don't need it. Perhaps the cartopy import could be made lazy, so that you don't get the warning unless you try to use USCOUNTIES or USSTATES? As a workaround, I have resorted to doing this:
import logging
logging.disable(logging.ERROR)
from metpy.io.nexrad import Level3File
logging.disable(logging.NOTSET)
  1. The import process is very slow. I have a particular script which takes 1.3 seconds to execute, of which 1.2 seconds is spent importing Level3File and 0.1 seconds is spent opening a level 3 file and doing something with the data. It's not totally obvious to me where this delay originates, but I would guess we end up importing some other modules which are not actually needed. Perhaps a lazy evaluation of whatever slows this down would be the best solution?
In [1]: time from metpy.io.nexrad import Level3File
Cannot import USCOUNTIES and USSTATES without Cartopy installed.
CPU times: user 1.14 s, sys: 89.6 ms, total: 1.23 s
Wall time: 1.23 s
@dlaw dlaw added the Type: Bug Something is not working like it should label Jun 8, 2021
@dopplershift dopplershift added this to the 1.1.0 milestone Jun 8, 2021
@dopplershift
Copy link
Member

@dlaw Thanks for reporting this. So with us dropping support for Python 3.7, we can finally use a module-level __getattr__() to handle access to USCOUNTIES and USSTATES and do much better with that warning, and we should do so.

However, the fact that your single import is even triggering that led me to dig in--we're doing something wrong. Importing from metpy.io is pulling in metar.py (fine), which then ends up triggering an import from metpy.plots (this is no good). Fixing that eliminated the warning AND dropped the import time for my system from ~1.37s to ~1.17s. I also found it was importing from metpy.calc--hiding that a bit dropped the time for from metpy.io import Level3File to ~1.05s on my system. I'll put in a PR with some of that for 1.1.

You might also see some benefit with using the current main branch, which made the loading of station data information a lazy process.

Also, FYI, we don't recommend the use of from metpy.io.nexrad import Level3File, but instead use from metpy.io import Level3File. The module layout below e.g. metpy.io is considered an implementation detail and is subject to change without warning.

dopplershift added a commit to dopplershift/MetPy that referenced this issue Jun 22, 2021
)

This avoids importing metpy.calc and metpy.plots in the metar parsing
code, which improves import times for code that's only doing unrelated
I/O, as well as eliminates a warning when CartoPy is not installed.

The wx_code_map and wx_code_to_numeric function should eventually be
moved to metar.py, but that's an API change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something is not working like it should
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants