ACT
ACT copied to clipboard
ENH: Use lazy loading for loading in modules
Currently, we eagerly import every module upon importing ACT (discovery, qc, etc.).
This means that when you call the
import act
line, it will run through all of the ACT code to prepare to use. This can lead to long import times, on the order of ~10 seconds.
We can cut this down quite a bit by using the SciPy SPEC 001, which outlines how to use the Lazy Loader package.
The only caveat here is that "Python 3.7, with PEP 562, introduces the ability to override module getattr and dir. In combination, these features make it possible to again provide access to submodules, but without incurring performance penalties."
Which indicated this will only work with Python 3.7+
I included a first cut of this within the PR, and removed our 3.6 linux test since the only way we can merge this is by losing that test.
Closes #62
@kenkehoe this might be what you've been looking for in terms of import speed/improvement
@mgrover1 Looks like coverage dropped too, maybe due to imports? colormap module etc
@mgrover1 Looks like coverage dropped too, maybe due to imports? colormap module etc
Yup - the coverage checks all the lines that are run. Since we are not running everything upon importing, we will need to add additional tests to cover everything (ex. colormap)
@zssherman similar to the xarray module, we do need to eagerly load in the colormaps for matplotlib
@zssherman these tests won't pass until it has the associated keys, which are only accessible when running on main (after the PR is merged)
@mgrover1 gotcha, do we wait until we figure out the python 3.6 ADC stuff?
Probably - that makes sense!
@mgrover1 @zssherman generally, the coverage has been about 1.5% less so there are still some areas where we are seeing a decrease in the coverage. Let me look into it a little more on coveralls and see what's happening
Pinging @kenkehoe here as well for awareness.
@AdamTheisen I'm not really sure. How does this change the use of the read_netcdf() function use?
@mgrover1 could you respond to Ken on this?
@AdamTheisen I'm not really sure. How does this change the use of the read_netcdf() function use?
Users can still use the same API
import act
act.io.read_netcdf()
@kenkehoe Is this what you were asking?
So, coveralls is saying there's a decrease in other modules but when I drill down into them there's no change. I think the biggest change is that the following are not being loaded up at all initially here which is dropping the coverage. As @mgrover1 noted, this should be resolved when the tests run on production. I do think we should get an ENG in with ARM before we merge this to make sure there are no larger issues to think about.
