On Fri, Nov 15, 2013 at 1:17 AM, Joachim Durchholz <[email protected]> wrote: > Just for reference: > The diagnostic code needs to be runnable from two entirely different > contexts. > One context is the bin/diagnose_imports program. It's intended to be run > interactively, when a developers wants to inspect import cycles and such. > The other context is sympy/utilities/tests/test_module_imports.py, which is > a unit test. It's a unit test that needs to be runnable both in Travis and > when run interactively as bin/test. It is unusual in that it must always run > a subprocess.
That's no big deal. The tests are run in a subprocess by default anyway. > > >>> Solution 1: Don't install SymPy as a Python module. Leave SymPy in >>> whatever directory Travis downloads from github and run the tests >>> from there; that's what developers do anyway. > > I think that > > >> Installing is important because we want to test that things get >> installed correctly. > > means that solution 1 is -1. > I wasn't too thrilled by that approach anyway :-) > > >>> Solution 2: Somehow tell the tests where the binaries (such as >>> isympy) are installed. Install the diagnostic script alongside >>> isympy. >>> This allows end users to run the tests as well, so I'd prefer that >>> approach; > > Any comments on this one? I'm -1 to installing test binaries for users. But maybe it could be non-default. > > I'd put the bin/ location in sympy.bin_directory; SymPy would default that > to a bin/ parallel to sympy/, but could be provided with a different > location either through an environment variable (e.g. $SYMPY_BIN_DIRECTORY) > or a command-line parameter (e.g. --bin-directory). > > >>> Solution 3: Move the diagnostic code into the sympy/utilities >>> directory. Let both unit test and bin/diagnose_imports import that >>> module. >>> >>> Make sure that utilities/__init__.py does NOT import the new diagnostic >>> Python code since it is not supposed to be part of SymPy. >>> On the plus side, this does not require any changes to SymPy's >>> infrastructure. >>> On the minus side, I don't really like adding a merely diagnostic module >>> outside a test/ directory. (Well, maybe the real problem is that bin/ >>> contains both test and production code.) > > >> Alternately, your solution 3 sounds fine to me. I'm not a huge fan of >> installing test functions into user-space. > > > I must be misunderstanding something here; solution 3 means that the > diagnostic code lives in sympy/utilities (to be called from either > sympy/utilities or sympy/utilities/test/test_module_imports, depending on > context). That last sentence was referring to solution 2. Sorry for the confusion. > >> [Solution 4:] >> >> I would just add it as another test that gets run with setup.py test >> (which calls some run_all_tests function). For Travis, you should run >> it separately, before it gets installed. Installing is important >> because we want to test that things get installed correctly. > > > That would mean it's not called from bin/test. > The consequence is that if a developer accidentaly introduces an import > problem, he won't see it until an hour later when Travis is done, and only > if they specifically look for the Travis results. That's frustration > (unexpected failures, can't add the pull request) and embarrassment (public > display of mistakes); such things can make new contributors stop > contributing. If it's any consolation, no one runs the full bin/test any more, unless they are pretty sure that they will need to. At least for me, I just test the things that are likely to be broken, and then let Travis do the rest. With that being said, I agree that having it as part of test_code_quality.py would be the most ideal. Aaron Meurer > > Also, I'd want to avoid having an entirely separate test code path just > for Travis. That's breakage waiting to happen whenever the Travis setup > changes, or whenever Sympy's test machinery changes. > I'd be +1 on this approach if separate programs were run as part of the > normal unit tests, but we don't have this, I'm not sure whether it's a good > idea to add it because it's sure going to complicate the test code which is > way too complicated and redundant already, and even if we want it I'm not > going to do it because I have not enough tolerance for yet another detour > before I can finally start work on eliminating C. Reading through your original email again, I think the best solution would be to move the code from bin/diagnose_imports to the library somewhere (utilities/diagnose.py or something, or even external/importtools.py). Then bin/diagnose_imports will be a simple wrapper. Your unit test could then just import that file and go. I suggested importtools.py btw because that file is guaranteed to not import SymPy (although I just noticed that it doesn't say that at the top of the file, so that should be fixed). I forgot the exact reason for that, but I seem to remember it was a good one. A separate file in utilities could also easily have this. Aaron Meurer -- You received this message because you are subscribed to the Google Groups "sympy" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/sympy. For more options, visit https://groups.google.com/groups/opt_out.
