Qcodes icon indicating copy to clipboard operation
Qcodes copied to clipboard

Reducing qcodes import times

Open edumur opened this issue 3 years ago • 10 comments

Hi all,

Following this discussion #4565 and this issue #4543 I am proposing this pull request to work on reducing qcodes loading time.

I have started with pandas. I tried to propagate what @jenshnielsen did to make pandas import lazy-import. I am not sure I have succeeded since I still see pandas being imported while doing importtime-waterfall. I would gladly improve the request if someone help me on this.

If this becomes a successful endeavor, I will continue by making other import lazy-import.

edumur avatar Sep 14 '22 08:09 edumur

And also, I do not no how to run qcodes test and check if I didn't break anything ^^'. Again, if someone may help me.

edumur avatar Sep 14 '22 08:09 edumur

Thanks, I will try to have a look asab and see if I can figure out why/if pandas is still being imported eagerly

jenshnielsen avatar Sep 14 '22 08:09 jenshnielsen

Found out that pandas was imported by xarray so I passed xarray in lazy import.

On the data_set file, I got some error and had to write the type of some parameters as 'xr.Dataset', see line 693 for example. I do not know why and if this is correct.

edumur avatar Sep 16 '22 08:09 edumur

Yep, forget what I wrote, it is fixed.

edumur avatar Sep 16 '22 08:09 edumur

Looks good. Did you verify that you need to also make changes to the tests. Importing qcodes should not import the tests module. If it does that seems like a bug we should also fix.

jenshnielsen avatar Sep 16 '22 08:09 jenshnielsen

You right, test file do not matters. Could you remind me how to run the test locally?

edumur avatar Sep 16 '22 09:09 edumur

Alright there is maybe the import of Monitor to work on but I am waiting opinion(s) from others. On my pc it looks like it is 20% faster, can anyone look on their machine?

edumur avatar Sep 16 '22 09:09 edumur

I should have include everything you said. Please tell me if I missed anything.

edumur avatar Sep 20 '22 07:09 edumur

Codecov Report

Merging #4616 (f8718ed) into master (81f1424) will decrease coverage by 0.07%. The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #4616      +/-   ##
==========================================
- Coverage   68.30%   68.23%   -0.08%     
==========================================
  Files         339      339              
  Lines       31974    31979       +5     
==========================================
- Hits        21841    21820      -21     
- Misses      10133    10159      +26     

codecov[bot] avatar Sep 20 '22 10:09 codecov[bot]

Should be good

edumur avatar Sep 29 '22 12:09 edumur

Done.

I did not get what you meant by "documentation about where to import it from"...

edumur avatar Oct 03 '22 06:10 edumur

Looks good. thanks @edumur

jenshnielsen avatar Oct 04 '22 11:10 jenshnielsen

bors merge

jenshnielsen avatar Oct 04 '22 11:10 jenshnielsen

:clock1: Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

bors[bot] avatar Oct 04 '22 11:10 bors[bot]

bors merge

jenshnielsen avatar Oct 04 '22 12:10 jenshnielsen