Skip to content

Conversation

@samuelgarcia
Copy link
Contributor

This PR remove unnecessary import at module level and import extra dependency only when they are needed when instanciating an IO class.

See #1160

This should:

  • help in complicated environement (by not importing everything)
  • decrease import duration

The drawback:

  • it raise error only when instanciate (run time) and not at load time.

@samuelgarcia
Copy link
Contributor Author

@alejoe91

@pep8speaks
Copy link

pep8speaks commented Aug 31, 2022

Hello @samuelgarcia! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-08-31 06:50:56 UTC

@JuliaSprenger
Copy link
Member

Hi @samuelgarcia
Thanks for the simplifications. Did you confirm this is reducing the import time?
I think we are loosing at tiny bit of user friendliness here, but it also simplifies the code, so let's go with it.

@apdavison apdavison merged commit 29e4c51 into NeuralEnsemble:master Aug 31, 2022
@JuliaSprenger
Copy link
Member

For reference: This change improves the import time on my system from 1.6s to 0.5s. @samuelgarcia It works 👍

@samuelgarcia
Copy link
Contributor Author

cool.
To be totaly honest I did not make benchmark :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants