fcurella / django-fakery

🏭 An easy-to-use implementation of Creation Methods for Django, backed by Faker.
http://django-fakery.readthedocs.org/en/stable/
MIT License
115 stars 6 forks source link

Tentative fix for Python 3 #33

Closed arnaudlimbourg closed 7 years ago

arnaudlimbourg commented 7 years ago

I closed the previous PR that had a borked merge/rebase.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 93.85% when pulling d5e0e392386f7c06b229f2d29464f17e4610fd93 on arnaudlimbourg:patch-2 into 4f83a54539350268331f33c69714bddbe271e4be on fcurella:master.

fcurella commented 7 years ago

@arnaudlimbourg could you post the full stacktrace?

If you're getting the exception because you're trying to use django.contrib.gis, but you have gdal misconfigured, then raising the exception is the correct behavior. But if you don't use anything related to django.contrib.gis, I'd like to figure out why the exception gets raised in the first place.

arnaudlimbourg commented 7 years ago

Here is the full stack trace. I don't use geo features and have not configured them, under my python2 virtualenv I do not have any issue.

It seems to happen only when I run tests with py.test though, runserver seems to work fine.

/usr/local/Cellar/python3/3.6.2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/inspect.py:79: in isclass
    return isinstance(object, type)
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/utils/functional.py:238: in inner
    self._setup()
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/utils/functional.py:386: in _setup
    self._wrapped = self._setupfunc()
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django_fakery/__init__.py:8: in <lambda>
    factory = SimpleLazyObject(lambda: import_string('django_fakery.faker_factory.factory'))
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/utils/module_loading.py:20: in import_string
    module = import_module(module_path)
../../.virtualenvs/python3venv/lib/python3.6/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:978: in _gcd_import
    ???
<frozen importlib._bootstrap>:961: in _find_and_load
    ???
<frozen importlib._bootstrap>:950: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:655: in _load_unlocked
    ???
<frozen importlib._bootstrap_external>:678: in exec_module
    ???
<frozen importlib._bootstrap>:205: in _call_with_frames_removed
    ???
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django_fakery/faker_factory.py:12: in <module>
    from .compat import string_types
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django_fakery/compat.py:24: in <module>
    HAS_GEOS = geos_version_info()['version'] >= '3.3.0'
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/geos/libgeos.py:193: in geos_version_info
    ver = geos_version().decode()
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/geos/libgeos.py:161: in __call__
    self.func = self.get_func(*self.args, **self.kwargs)
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/geos/libgeos.py:165: in get_func
    from django.contrib.gis.geos.prototypes.threadsafe import GEOSFunc
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/geos/prototypes/threadsafe.py:3: in <module>
    from django.contrib.gis.geos.base import GEOSBase
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/geos/__init__.py:5: in <module>
    from .collections import (  # NOQA
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/geos/collections.py:11: in <module>
    from django.contrib.gis.geos.geometry import GEOSGeometry, LinearGeometryMixin
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/geos/geometry.py:11: in <module>
    from django.contrib.gis import gdal
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/gdal/__init__.py:28: in <module>
    from django.contrib.gis.gdal.datasource import DataSource
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/gdal/datasource.py:39: in <module>
    from django.contrib.gis.gdal.driver import Driver
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/gdal/driver.py:5: in <module>
    from django.contrib.gis.gdal.prototypes import ds as vcapi, raster as rcapi
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/gdal/prototypes/ds.py:9: in <module>
    from django.contrib.gis.gdal.libgdal import GDAL_VERSION, lgdal
../../.virtualenvs/python3venv/lib/python3.6/site-packages/django/contrib/gis/gdal/libgdal.py:45: in <module>
    % '", "'.join(lib_names)
E   django.core.exceptions.ImproperlyConfigured: Could not find the GDAL library (tried "gdal", "GDAL", "gdal2.1.0", "gdal2.0.0", "gdal1.11.0", "gdal1.10.0", "gdal1.9.0"). Is GDAL installed? If it is, try setting GDAL_LIBRARY_PATH in your settings.
twidi commented 7 years ago

Note that I have the exact same error without initializing django, ie when running ipython and then from django.contrib.gis.geos.libgeos import geos_version_info. (I tested with python 2.7.12, django 1.11.5)

So yes, this line can cause the ImproperlyConfigured exception to be raised, and as we don't do it in our own code, but it's done in fakery, fakery should manage it.

Also, important: this problem does not exist with django 1.10.

twidi commented 7 years ago

Also if I run this command twice (django.contrib.gis.geos.libgeos import geos_version_info) in ipython, I have the ImproperlyConfigured the first time, then an ImportError the second time......

twidi commented 7 years ago

So with this discovery, here is how I manage this problem in my code:

from django.core.exceptions import ImproperlyConfigured
try:
    from django.contrib.gis.geos.libgeos import geos_version_info
except ImproperlyConfigured:
    pass

from django_fakery import factory
fcurella commented 7 years ago

I'm not entirely convinced the error should be swallowed. I'm all for easy of use, but if the ImproperlyConfigured exception is Django's way of telling you that you should have initialized your settings, we should leave it that way, or possibly re-raise it with a more descriptive message.

arnaudlimbourg commented 7 years ago

what I don't understand is that since I don't use it I don't see why I should configure it in the first place. Since it doesn't happen on Django 1.10 I wonder what changed.

fcurella commented 7 years ago

@arnaudlimbourg You shouldn't need to configure libgeos, but you should need to initialize Django.

I'm assuming you're using pytest-django with pytest, which should take care of that for you. I just want to make sure we are not always silencing an exception that's useful in some scenario.

twidi commented 7 years ago

In seems that in mpytest(-django) import the files to know if it has tests to run. In which file we import fakery, so it is done before initializing django. Maybe we have something wrong in your code that is doing this.

Just to say that I understand your point. I move on on this now that I found a workaround.

Thanks for your library, keep up the good work.

fcurella commented 7 years ago

@arnaudlimbourg @twidi would you mind trying my solution on the compat branch? https://github.com/fcurella/django-fakery/pull/34

fcurella commented 7 years ago

Closed by #34