Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

58 nf speedup cli #467

Merged
merged 3 commits into from
Oct 30, 2017
Merged

58 nf speedup cli #467

merged 3 commits into from
Oct 30, 2017

Conversation

forman
Copy link
Member

@forman forman commented Oct 30, 2017

This partly addresses #58, as cate --version and cate -h now perform much faster.

Performance improvement comes from two optimisations:

  1. cate.util does not aggregate sub-packages anymore. We have to explicitly import now. This is because some sub-packages imports are slow, notably the new cate.util.opimpl.
  2. CLI commands in cate.cli.main now only import required modules when they are executed.

@codecov-io
Copy link

codecov-io commented Oct 30, 2017

Codecov Report

Merging #467 into master will decrease coverage by 0.01%.
The diff coverage is 81.25%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #467      +/-   ##
=========================================
- Coverage   76.92%   76.9%   -0.02%     
=========================================
  Files          74      74              
  Lines       10438   10497      +59     
=========================================
+ Hits         8029    8073      +44     
- Misses       2409    2424      +15
Impacted Files Coverage Δ
cate/util/cache.py 81.78% <100%> (+0.07%) ⬆️
cate/webapi/websocket.py 31.39% <100%> (+0.4%) ⬆️
cate/ops/outliers.py 100% <100%> (ø) ⬆️
cate/ops/correlation.py 98.91% <100%> (ø) ⬆️
cate/core/op.py 91.98% <100%> (+0.12%) ⬆️
cate/core/workflow.py 93.58% <100%> (+0.02%) ⬆️
cate/core/workspace.py 87.12% <100%> (+0.13%) ⬆️
cate/webapi/wsmanag.py 28.49% <100%> (+0.4%) ⬆️
cate/core/wsmanag.py 65.32% <100%> (+0.25%) ⬆️
cate/util/process.py 90.15% <100%> (ø) ⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84e0563...fb52cde. Read the comment docs.

Copy link
Member

@JanisGailis JanisGailis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a general comment. Is there a reason why we use relative imports in some places?

As for this PR - looks good.

Copy link
Collaborator

@mzuehlke mzuehlke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speedup is always a good thing.
All the new flake8 warnings are from flake8 3.5.0. a new version with new checks.
We can fix these later.

@forman
Copy link
Member Author

forman commented Oct 30, 2017

@JanisGailis we prefer relative imports over absolute ones because they do not force loading the parent package's imports from ./__init__.py.

@forman forman merged commit e7f3a39 into master Oct 30, 2017
@forman forman deleted the 58-nf-speedup_cli branch December 8, 2017 14:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants