Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

warning that duecredit is not installed when loading package #1872

Closed
kain88-de opened this issue Apr 23, 2018 · 10 comments
Closed

warning that duecredit is not installed when loading package #1872

kain88-de opened this issue Apr 23, 2018 · 10 comments

Comments

@kain88-de
Copy link
Member

Expected behaviour

no warning when loading the module

Actual behaviour

warning that duecredit is not installed.

Currently version of MDAnalysis:

(run python -c "import MDAnalysis as mda; print(mda.__version__)")
0.18.0

@orbeckst
Copy link
Member

I thought we had taken care of this as part of PR #1822 , specifically e3d5cac but it seems we have to revisit.

@orbeckst
Copy link
Member

For the record (using Python 3.6 on macOS) in a conda environment without duecredit gives:

cat > credit.py << 'EOF'
import MDAnalysis
print(MDAnalysis.__version__)
EOF
python credit.py

gives

/[...]/mdanalysis/package/MDAnalysis/due.py:88: UserWarning: No module named 'duecredit'
  warnings.warn(str(err))
0.18.1-dev

@orbeckst
Copy link
Member

orbeckst commented Apr 23, 2018

It raises a ModuleNotFoundError: No module named 'duecredit' (since Python 3.6). Will patch it. Not sure how to test it in our current testing framework because the due.py code is run on the very first import of MDAnalysis. Maybe mock duecredit and re-import MDAnalysis.due... suggestions welcome.

EDIT: removed stuff on how I was to dumb to check if ModuleNotFoundError is an instance of ImportError.

@richardjgowers
Copy link
Member

@orbeckst we used to have some tests that tested for deprecation warnings or something, I think we deleted the dict entry from sys.modules and that forced the reload of the module

@orbeckst
Copy link
Member

orbeckst commented Apr 23, 2018

Btw, the Python docs say https://docs.python.org/3/library/exceptions.html#ModuleNotFoundError

exception ModuleNotFoundError

A subclass of ImportError which is raised by import when a module could not be located. It is also raised when None is found in sys.modules.

New in version 3.6.

The following does catch that ModuleNotFoundError

In [12]: try:
    ...:     raise ModuleNotFoundError("foo")
    ...: except ImportError as err:
    ...:     print("Import error", err)
    ...:
Import error foo

@orbeckst
Copy link
Member

I just can't read code anymore... the exceptions work as they should, I just have to remove the debugging code :-p

warnings.warn(str(err))

orbeckst added a commit that referenced this issue Apr 23, 2018
- fix #1872
- Do not show a warning if duecredit is not installed;
  if users want the functionality they can install duecredit
  but we should not bother all others.
- NOTE: This was manually tested in a local installation without
        duecredit but doing a real unit test is difficult and
        has not been attempted.
orbeckst added a commit that referenced this issue Apr 23, 2018
- fix #1872
- Do not show a warning if duecredit is not installed;
  if users want the functionality they can install duecredit
  but we should not bother all others.
- NOTE: This was manually tested in a local installation without
        duecredit but doing a real unit test is difficult and
        has not been attempted.
@mimischi
Copy link
Contributor

I'm still getting the same warning with MDAnalysis 0.18.0 in a fresh environment:

$ python -c "import MDAnalysis as mda; import sys; print(mda.__version__); print(sys.version);"
/home/mischi/.virtualenvs/test-oN5qujzB/lib/python3.6/site-packages/MDAnalysis/due.py:88: UserWarning: No module named 'duecredit'
  warnings.warn(str(err))
0.18.0
3.6.5 (default, Apr 14 2018, 13:17:30) 
[GCC 7.3.1 20180406]

@kain88-de
Copy link
Member Author

This error is fixed in the next release

@Ruibin-Liu
Copy link

I first saw the same warning. But after I install the duecredit package, the warning message disappears. I guess the package dependencies are not right?

@richardjgowers
Copy link
Member

@rainbeeliu the dependencies are correct in that duecredit isn't necessary for the package to function. You only need duecredit for duecredit (citation) things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants