cannot import name 'memoize' from 'kwant._common'

[digdig]

I am using windows 10 and anaconda with python 3.9.

I have installed kwant in anaconda, then I install kpm-tools using pip install. However, when import kpm_tools, the following errors occurs, then I tried to solve the problem by pip install memoize, the install is successful but the same error is still present.

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\xxxx\.conda\envs\kpm\lib\site-packages\kpm_tools\__init__.py", line 3, in <module>
    from .bloch import _hopping_distance
  File "C:\Users\xxxx\.conda\envs\kpm\lib\site-packages\kpm_tools\bloch.py", line 13, in <module>
    from kwant._common import memoize
ImportError: cannot import name 'memoize' from 'kwant._common' (C:\Users\xxxx\.conda\envs\kpm\lib\site-packages\kwant\_common.py)

[grothesque]

The problem is that KPM-tools is trying to use a non-public interface of a hidden submodule of an unreleased version of Kwant... And it is doing this without even declaring Kwant as its dependency.

While Python does not enforce interfaces, this is not something that a publicly released library should do.

Instead of relying on non-public interfaces, KPM-tools should rather simply take over the functionality it wants from Kwant. It should so so while respecting Kwant's licence, i.e. acknowledging that fact in the copyright notice of the relevant file and in the license.

@digdig, as a workaround you can try to install the development version of Kwant. You will have to compile it yourself. Or you can try to modify KPM-tools so that it works with stable Kwant.

[piskunow]

Hi @digdig , thanks for opening this issue! Indeed, you would need the Kwant development version installed. I will clarify this in the installation instructions.

Like @grothesque points out, this is not the standard approach, however, development versions can be listed as dependencies. I agree this is not ideal, and hopefully in the future there will be a new release of Kwant.

For the time being, I can add the dependency pointing to the "master" branch or the latest commit where all tests of kpm-tools passed, as

[tool.poetry.dependencies]
kwant = { git = "https://gitlab.kwant-project.org/kwant/kwant.git", branch = "master" }

and point the install all the dependencies that building Kwant from source requires.

Happy to receive any feedback, and thanks to you for your constructive criticism and for keeping a respectful tone.

[digdig]

Thank you @grothesque for point out the core of the problem.

[digdig]

Hi @digdig , thanks for opening this issue! Indeed, you would need the Kwant development version installed. I will clarify this in the installation instructions.

Like @grothesque points out, this is not the standard approach, however, development versions can be listed as dependencies. I agree this is not ideal, and hopefully in the future there will be a new release of Kwant.

For the time being, I can add the dependency pointing to the "master" branch or the latest commit where all tests of kpm-tools passed, as

[tool.poetry.dependencies]
kwant = { git = "https://gitlab.kwant-project.org/kwant/kwant.git", branch = "master" }

and point the install all the dependencies that building Kwant from source requires.

Happy to receive any feedback, and thanks to you for your constructive criticism and for keeping a respectful tone.

Thanks for the reply. I think it will be better if you can build kpm-tools upon the stable version of kwant. That would lead to a wider potential users. After all, building from source is a rather complicated task...

[grothesque]

Like @grothesque points out, this is not the standard approach, however, development versions can be listed as dependencies. I agree this is not ideal, and hopefully in the future there will be a new release of Kwant.

I hope so as well, but even then kwant._common.memoize will remain doubly non-public: memoize is not listed in kwant._common.__all__, and then in addition kwant._common is for internal use only.

[piskunow]

Thank you both for raising the issue and pointing the core problem. I have addressed them with #72 and will make a release with the changes. Feel free to test and raise a new issue if this or other problems appear. Furthermore, any contribution that addresses an issue is also welcome!