genenetwork / genenetwork2

GeneNetwork (2nd generation)
http://gn2.genenetwork.org/
GNU Affero General Public License v3.0
34 stars 24 forks source link

Bug: Remove circular imports #408

Closed BonfaceKilz closed 4 years ago

BonfaceKilz commented 4 years ago

Description

We have a bunch of circular imports, which are caught during unittests.

Steps to reproduce

  1. cd into the top-level directory
  2. Run: env GN2_PROFILE=/home/bonface/opt/genenetwork2 TMPDIR=/home/bonface/tmp SERVER_PORT=5004 GENENETWORK_FILES=/home/bonface/gnu/data/gn2_data/ /home/bonface/projects/genenetwork2/bin/genenetwork2 /home/bonface/projects/genenetwork2/etc/default_settings.py -c to get into a python REPL that has the correct variables set
  3. Try: import utility.tools
Click to toggle stack trace
>>> import utility.tools
Traceback (most recent call last):
  File "", line 1, in 
  File "utility/tools.py", line 8, in 
    from wqflask import app
  File "wqflask/__init__.py", line 23, in 
    from wqflask.api import router
  File "wqflask/api/router.py", line 16, in 
    from wqflask.api import correlation, mapping, gen_menu
  File "wqflask/api/correlation.py", line 11, in 
    from base import data_set
  File "base/data_set.py", line 22, in 
    from db.call import fetchall, fetchone, fetch1
  File "db/call.py", line 8, in 
    from utility.tools import USE_GN_SERVER, LOG_SQL, GN_SERVER_URL
ImportError: cannot import name USE_GN_SERVER
zsloan commented 4 years ago

It might be worth asking Pjotr about this if you run into any issues, since I think he wrote the /bin/genenetwork shell script and most of utility/tools.py

My guess is that he was having to work around these issues when he added the -c tag.

On Tue, Jul 21, 2020 at 3:06 PM BonfaceKilz notifications@github.com wrote:

Description

We have a bunch of circular imports, which are caught during unittests. Steps to reproduce

  1. cd into the top-level directory
  2. Run: env GN2_PROFILE=/home/bonface/opt/genenetwork2 TMPDIR=/home/bonface/tmp SERVER_PORT=5004 GENENETWORK_FILES=/home/bonface/gnu/data/gn2_data/ /home/bonface/projects/genenetwork2/bin/genenetwork2 /home/bonface/projects/genenetwork2/etc/default_settings.py -c to get into a python REPL that has the correct variables set
  3. Try: import utility

Click to toggle stack trace

import utility Traceback (most recent call last): File "", line 1, in File "utility/init.py", line 36, in import tools File "utility/tools.py", line 8, in from wqflask import app File "wqflask/init.py", line 23, in from wqflask.api import router File "wqflask/api/router.py", line 16, in from wqflask.api import correlation, mapping, gen_menu File "wqflask/api/correlation.py", line 11, in from base import data_set File "base/data_set.py", line 22, in from db.call import fetchall, fetchone, fetch1 File "db/call.py", line 8, in from utility.tools import USE_GN_SERVER, LOG_SQL, GN_SERVER_URL ImportError: cannot import name USE_GN_SERVER

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/genenetwork/genenetwork2/issues/408, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANQJGD4YT2ZZZOYFQ2L7GTR4XYNFANCNFSM4PEAC2FA .

BonfaceKilz commented 4 years ago

Here's a useful link: https://stackabuse.com/python-circular-imports/

BonfaceKilz commented 4 years ago

Found a fix and a crude explanation for what's going on :smile:

So there's need to have: from wqflask import app. Here's what happens when you don't have it:

import unittest
import base.data_set

class TestDataSet(unittest.TestCase):
    def test_random_test(self):
        self.assertEqual(1, 1)
Click to toggle stack trace
======================================================================
ERROR: tests.base.test_data_set (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: tests.base.test_data_set
Traceback (most recent call last):
  File "/gnu/store/cj2xkwigzcdd174hpanj18dp2p39mprv-python2-2.7.17/lib/python2.7/unittest/loader.py", line 254, in _find_tests
    module = self._get_module_from_name(name)
  File "/gnu/store/cj2xkwigzcdd174hpanj18dp2p39mprv-python2-2.7.17/lib/python2.7/unittest/loader.py", line 232, in _get_module_from_name
    __import__(name)
  File "tests/base/test_data_set.py", line 4, in 
    import base.data_set
  File "base/data_set.py", line 22, in 
    from db.call import fetchall, fetchone, fetch1
  File "db/call.py", line 8, in 
    from utility.tools import USE_GN_SERVER, LOG_SQL, GN_SERVER_URL
  File "utility/tools.py", line 8, in 
    from wqflask import app
  File "wqflask/__init__.py", line 23, in 
    from wqflask.api import router
  File "wqflask/api/router.py", line 16, in 
    from wqflask.api import correlation, mapping, gen_menu
  File "wqflask/api/correlation.py", line 11, in 
    from base import data_set
ImportError: cannot import name data_set

----------------------------------------------------------------------
Ran 16 tests in 0.001s

Now after adding the import:

import unittest

from wqflask import app
import base.data_set

class TestDataSet(unittest.TestCase):
    def test_random_test(self):
        self.assertEqual(1, 1)

The tests now pass:


----------------------------------------------------------------------
Ran 16 tests in 0.001s

OK

Since the utility.tools module is coupled to the app object, we need to keep reimporting it. I'm sure there's a better way to expose it- I did it in a previous lifetime, but can't recall how to(something to do with how you structure your flask app)

BonfaceKilz commented 4 years ago

Now at this point we can't radically re-structure things so I'm going ahead to close this.

zsloan commented 4 years ago

With utilities/tools.py specifically, I don't think it even existed for early GN2; I think Pjotr may have added it in the process of setting up GUIX or something (and also changing things that were previously hardcoded in a config file to instead be pulled from environment variables).

On Wed, Jul 22, 2020, 3:26 PM BonfaceKilz notifications@github.com wrote:

Closed #408 https://github.com/genenetwork/genenetwork2/issues/408.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/genenetwork/genenetwork2/issues/408#event-3577430345, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANQJGCASZ25ND2FZDSH4JTR45DN7ANCNFSM4PEAC2FA .

robwwilliams commented 4 years ago

Thanks Bonface. Great contributions!

On Wed, Jul 22, 2020 at 3:26 PM BonfaceKilz notifications@github.com wrote:

Now at this point we can't radically re-structure things so I'm going ahead to close this.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/genenetwork/genenetwork2/issues/408#issuecomment-662678051, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC55V7AVGOOPB4KS6PQHV7TR45DN7ANCNFSM4PEAC2FA .

-- Rob

Robert W. Williams, Ph.D. Chair: Department of Genetics, Genomics and Informatics University of Tennessee Health Science Center 71 S Manassas St, Memphis TN 38103-3310, TSRB Room 405 Tele/Cell: 901 604 4752 EMAIL: rwilliams@uthsc.edu or labwilliams@gmail.com