Add apt parser sub_package

[ccossou]

In this branch, I added my apt_parser package into MiriTE.

First commit, I just add the corresponding file
Second commit, I include apt_parser into the setup.py to make it installable.

See http://www.miricle.org/bugzilla/show_bug.cgi?id=573 for context.

The original package is here: https://git.ias.u-psud.fr/JWST/apt_parser

[smbeard]

Christophe, I tried your new branch but when I ran the unit tests the test_templates.py module failed with the following error message:

$ python test_templates.py
Running test_MiriCoron
Running test_MiriCpc
Running test_MiriDark
Running test_MiriExternalFlat
Running test_MiriImaging
Running test_MiriLRS
Running test_MiriMRS
Running test_MiriMRSCrossGratingEngineering
Running test_MiriMRS_parallel
(miricle.devel) smb@jura:/sw4/smb/MIRI-GIT/MiriTE/apt_parser/tests$ python test_dithering.py
Running test_dithering
Traceback (most recent call last):
File "test_dithering.py", line 71, in
func()
TypeError: test_dithering() missing 2 required positional arguments: 'sample_xml' and 'expected'

[ccossou]

I can't reproduce your bug. But I found other problems when trying to reproduce: My imports were still using the old package instead of the new one, since both were installed in the same environment (miricle.devel).

I then created a specific virtual envs to test it. But I realized that python setup.py develop doesn't work with this package, do you know if there's a way to use it without having to install everything each time I change something? Since the "miri" level doesn't exist, every symbolic link fail to reproduce the package structure, and "import miri" doesn't work. I'm sure you must have more experience than me on this.

(MiriTE) laptop.login:MiriTE(add_apt_parser)$ pytest apt_parser/
============================= test session starts ==============================
platform linux -- Python 3.6.8, pytest-5.0.1, py-1.8.0, pluggy-0.12.0
rootdir: /media/shared/miricle/MiriTE
collected 22 items / 2 skipped / 20 selected                                   

apt_parser/tests/test_dithering.py ....                                  [ 18%]
apt_parser/tests/test_library.py ........                                [ 54%]
apt_parser/tests/test_templates.py .........                             [ 95%]
apt_parser/tests/test_utils.py .                                         [100%]

===================== 22 passed, 2 skipped in 0.35 seconds =====================

I tried nosetests, to try and reproduce the bug and got:

(MiriTE) laptop.login:MiriTE(add_apt_parser)$ nosetests apt_parser/
Usage: nosetests [options]

nosetests: error: Error reading config file 'setup.cfg': no such option 'with-xcoverage'

What test suite do you use, and what command do you use in jenkins? 

[ccossou]

I think I've found why the test fail when you try to launch it. You try to launch it directly with Python:

(MiriTE) laptop.login:MiriTE(add_apt_parser)$ python apt_parser/tests/test_dithering.py 
Running test_dithering
Traceback (most recent call last):
  File "apt_parser/tests/test_dithering.py", line 71, in <module>
    func()
TypeError: test_dithering() missing 2 required positional arguments: 'sample_xml' and 'expected'

while if you try with pytest, it works:

(MiriTE) laptop.login:MiriTE(add_apt_parser)$ pytest apt_parser/tests/test_dithering.py 
============================= test session starts ==============================
platform linux -- Python 3.6.8, pytest-5.0.1, py-1.8.0, pluggy-0.12.0
rootdir: /media/shared/miricle/MiriTE
collected 4 items                                                              

apt_parser/tests/test_dithering.py ....                                  [100%]

=========================== 4 passed in 0.19 seconds ===========================

[smbeard]

Christophe,
Yes, I was launching the test directly with Python. The other unit tests in MiriTE don't depend on pytest, so they can be launched with pytest, nosetests or directly and the result is the same. I launch a test directly with Python when I want to run only the tests within one module.

This shouldn't be a problem in the short term, since Jenkins is configured to use pytest. Is it possible to trap the exception and report a more informative error message (such as "Please launch this test using pytest, since it depends on pytest utilities") so that if Jenkins is modified in the future, or if anyone else tries to run the test directly, as I did, they will know what is happening.

[ccossou]

Steven,

To be honest, I don't know how to change my test to put an error message. Because the error comes from internal code that is launched with the command:

python test_dithering.py

I don't know what exactly python is doing when this command is ran. At the start of my script, there is an import pytest that should fail if pytest is not installed though.

If you have an idea, I'm open to suggestions.

Finally, I don't even know if nosetest fail to launch this test. When I tried, I get an error with setup.cfg unknown option "with-xcoverage"

[smbeard]

Christophe,

By the way, it is only the test_dithering.py module which fails. The other modules successfully run their tests when I launch them with, for example

python test_templates.py

What's the difference between test_dithering.py and the other modules? If you could give test_dithering.py the same structure as the other modules, the issue goes away.

The test fails at line 71 of test_dithering.py, so the kind of trap I am thinking of would be something like this:

print("Running {}".format(func.name))
try:
func()
except TypeError as e:
strg = "TypeError exception while running test:\n\t%s" % str(e)
strg += "NOTE: This test depends on pytest utilities. Please launch with pytest."
raise TypeError(strg)

It reports the exception but adds a suggestion to try again using pytest.

[smbeard]

The "unknown option "with-xcoverage" error is an entirely separate issue. The following advice is contained in the README file:

TESTING

The unit tests included with the MIRI software can be run from the
top-level directory using nosetests or pytest by running either of
the commands

nosetests
pytest

from the top-level installation directory. If nosetests complains about
the --with-xcoverage parameter not being recognised, try the command

conda install nosexcover

Individual tests can also be run from within the "tests" directory.
For example:

cd MiriTE/datamodels/tests
python test_measured_model.py

[ccossou]

I've installed nosexcover and nosetests still gives me the same error:

(MiriTE) IAS-laptop.ccossou:MiriTE(add_apt_parser)$ nosetests apt_parser/
Usage: nosetests [options]

nosetests: error: Error reading config file 'setup.cfg': no such option 'with-xcoverage'

I'm using Python 3.6.8, nosexcover v1.0.11 and nosetests v1.3.7

---

Now I understand the problem for the tests. It all comes from the if __name__ == '__main__' part at the end of the code. I'd forgotten about that because I don't use that anymore in my tests, because of such problems. Ok sure, I'll add the test you provide. It make complete sense in that case.

[ccossou]

I've updated the test. But this will not be used with nosetests.

I've looked at alternative for parametrize tests with nosetests, with nothing obvious as a replacement.

I've also tried to run nosetests on that particular test, but it says that "miri" doesn't exist while I can import it in python. I'm not sure to understand this error:

(MiriTE) laptop.login:MiriTE(add_apt_parser)$ nosetests apt_parser/tests/test_dithering.py 
E
======================================================================
ERROR: Failure: ModuleNotFoundError (No module named 'miri')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/nose/failure.py", line 39, in runTest
    raise self.exc_val.with_traceback(self.tb)
  File "/usr/local/lib/python3.6/dist-packages/nose/loader.py", line 418, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/local/lib/python3.6/dist-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/local/lib/python3.6/dist-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/usr/lib/python3.6/imp.py", line 235, in load_module
    return load_source(name, filename, file)
  File "/usr/lib/python3.6/imp.py", line 172, in load_source
    module = _load(spec)
  File "<frozen importlib._bootstrap>", line 684, in _load
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/media/shared/miricle/MiriTE/apt_parser/tests/test_dithering.py", line 3, in <module>
    from miri.apt_parser.utils import assertDictEqual, read_test_xml, get_namespace
ModuleNotFoundError: No module named 'miri'

----------------------------------------------------------------------
Ran 1 test in 0.001s

FAILED (errors=1)
(MiriTE) laptop.login:MiriTE(add_apt_parser)$ python
Python 3.6.8 (default, Jan 14 2019, 11:02:34) 
[GCC 8.0.1 20180414 (experimental) [trunk revision 259383]] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import miri
>>> 

[vcgeers]

I would be cautious about error trapping in unit test modules, since it could interfere with the purpose of unit tests.

You probably want to make sure to re-raise the exact same kind of error, including the original traceback. You probably also should test what happens if an actual error of that kind were to occur in the module that the unit test is covering.

[ccossou]

No problem with the catch, it happens only in the if __name__ == '__main__': (that I forgot existed). So it only happens if one uses:

python test_dithering.py

When using pytest, it shouldn't reach that part, and fail beautifully if needs be.

[smbeard]

I would be cautious about error trapping in unit test modules, since it could interfere with the purpose of unit tests.

You probably want to make sure to re-raise the exact same kind of error, including the original traceback. You probably also should test what happens if an actual error of that kind were to occur in the module that the unit test is covering.

That's a good point. It was the reason why I reraised the same exception and included the "str(e)" in the exception message to display the actual exception. Would the following code provide a safer solution?

(import logging and create logger)

try:
func()
except TypeError as e:
logger.error("NOTE: This test depends on pytest utilities. Please launch with pytest.")
raise

This code traps the exception, displays a log message giving the user a hint and reraises the same exception.

[smbeard]

I found a typo in setup.py, which contains the following lines when defining the packages:

           'miri.apt_parser':'apt_parser',
           'miri.apt_parser.mirisim':'apt_parser/mirisim',

and should contain the following lines:

           'miri.apt_parser':'miri/apt_parser',
           'miri.apt_parser.mirisim':'miri/apt_parser/mirisim',

The "miri" level is missed from the package definition. This may be causing your "No module named 'miri'" error message.

[ccossou]

Originally, this was like this, but when i tried to install the package, I got the message "miri/" doesn't exist. I found that removing this solved my issue, which is why I removed it in a later commit.

If I add what you propose, I get the following error when trying to install miri:

(MiriTE) IAS-laptop.ccossou:MiriTE(add_apt_parser)$ python setup.py install
CONDA_PREFIX is 
[...]
running install
running bdist_egg
running egg_info
writing miri.egg-info/PKG-INFO
writing dependency_links to miri.egg-info/dependency_links.txt
writing top-level names to miri.egg-info/top_level.txt
error: package directory 'miri/apt_parser' does not exist

[smbeard]

I pulled your latest updates and the package now appears to be working. I can build MiriTE successfully and run pytest. The package definition strings contain a "miri." as expected, so my previous comment was a mistake. You already made the changes necessary to setup.py.

[smbeard]

For information, when I run the 4 test scripts directly using Python I get the following behaviour:

$ python test_library.py
$ python test_utils.py
$

$ python test_templates.py
Running test_MiriCoron
Running test_MiriCpc
Running test_MiriDark
Running test_MiriExternalFlat
Running test_MiriImaging
Running test_MiriLRS
Running test_MiriMRS
Running test_MiriMRSCrossGratingEngineering
Running test_MiriMRS_parallel

$ python test_dithering.py
Running test_dithering
Traceback (most recent call last):
File "test_dithering.py", line 72, in
func()
TypeError: test_dithering() missing 2 required positional arguments: 'sample_xml' and 'expected'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "test_dithering.py", line 76, in
raise Exception(strg) from e
Exception: pytest.parametrize not supported.
NOTE: This test depends on pytest utilities. Please launch with pytest.

Two scripts give no output, one works, and it is only test_dithering.py which fails. The latest error message gives more information. It's an attempt to access pytest.parameterize which fails. Perhaps test_dithering.py is the only module to use that facility? I am wondering if this is a miricle/conda compatibility issue?

[ccossou]

This is not a miricle/conda compatibility issue.

When you do:

python test_dithering.py

you essentially use a piece of code I wrote to launch the test manually. This piece of code doesn't handle parameters, and simply fail. This is not a problem of Pytest, or Nosetests, or Python. This is just a bad piece of code I wrote month ago without ever using it after.

I would be in favor of just deleting this piece of code. We're not supposed to launch unit test like this and I since then understood that. I don't write that anymore in any of my unit tests, but it somehow got stuck here.

The two file you mention that don't have any output simply don't do anything when you attempt to run them, because they don't have my "bad piece of code" in it. When run with pytest test_library.py they work properly.

[smbeard]

My mistake. I just noticed the "pytest.parametrize not supported" message is something your code reports, rather than something coming from within pytest itself.

[ccossou]

I checked and the parameterized package exist. It's supposed to work with nosetests, pytest and unittest. Unfortunately the current version doesn't work with pytest because yield is deprecated in pytest 4. That means all parameterized tests are skipped for now. I don't know when they expect to fix the package.

A branch pytest4 exist for solving that issue. It seems adressed but is not yet merged into trunk (wolever/parameterized#34 ).

Once it works, a fix would be:

from parameterized import parameterized

@parameterized(testdata)
def test_dithering(sample_xml, expected):

[ccossou]

@smbeard : Do I need to do something about this branch? Do I need to rewrite completely the parameterized test to a manual loop, even if it's not ideal, just to make the test work in this framework?

[smbeard]

Sorry about the long delay in replying. When I run nosetests after building the apt_parser branch the test fails with the following error messages:

EE.EEE...E.......

ERROR: Test of dithering

Traceback (most recent call last):
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
self.test(*self.arg)
TypeError: test_dithering() missing 2 required positional arguments: 'sample_xml' and 'expected'

======================================================================
ERROR: Read an XML string, adding the generic namespace for the JWST APT in it and any other parts needed

Traceback (most recent call last):
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
self.test(*self.arg)
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/util.py", line 620, in newfunc
return func(*arg, **kw)
TypeError: read_test_xml() missing 1 required positional argument: 'txt'

======================================================================
ERROR: From Proposal ID 1024 (1st target)

Traceback (most recent call last):
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
self.test(*self.arg)
TypeError: test_get_target() missing 2 required positional arguments: 'sample_xml' and 'expected'

======================================================================
ERROR: Read an XML string, adding the generic namespace for the JWST APT in it and any other parts needed

Traceback (most recent call last):
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
self.test(*self.arg)
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/util.py", line 620, in newfunc
return func(*arg, **kw)
TypeError: read_test_xml() missing 1 required positional argument: 'txt'

======================================================================
ERROR: test_library.test_get_prediction

Traceback (most recent call last):
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
self.test(*self.arg)
TypeError: test_get_prediction() missing 2 required positional arguments: 'input' and 'expected'

======================================================================
ERROR: Read an XML string, adding the generic namespace for the JWST APT in it and any other parts needed

Traceback (most recent call last):
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
self.test(*self.arg)
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/util.py", line 620, in newfunc
return func(*arg, **kw)
TypeError: read_test_xml() missing 1 required positional argument: 'txt'

---

Ran 17 tests in 1.228s

FAILED (errors=6)

I am not sure why you can successfully run the tests and I can't. You could work around this problem in the short term by renaming your test files so they are tmp_dithering.py tmp_library.py tmp_templates.py and tmp_utils.py. This will allow the branch to be merged with the trunk without breaking the Jenkins build. You could then work on the testing issue separately.

I also spotted a circular reference to mirisim which caused this failure to be reported:

======================================================================
ERROR: Failure: ImportError (No module named 'mirisim')

Traceback (most recent call last):
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/failure.py", line 39, in runTest
raise self.exc_val.with_traceback(self.tb)
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/loader.py", line 418, in loadTestsFromName
addr.filename, addr.module)
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/importer.py", line 47, in importFromPath
return self.importFromDir(dir_path, fqname)
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/site-packages/nose/importer.py", line 94, in importFromDir
mod = load_module(part_fqname, fh, filename, desc)
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/imp.py", line 245, in load_module
return load_package(name, filename)
File "/sw4/smb/anaconda3/envs/miricle.devel/lib/python3.5/imp.py", line 217, in load_package
return _load(spec)
File "", line 693, in _load
File "", line 673, in _load_unlocked
File "", line 697, in exec_module
File "", line 222, in _call_with_frames_removed
File "/sw/sw4/smb/MIRI-GIT/MiriTE/apt_parser/mirisim/init.py", line 3, in
from .run_sim import *
File "/sw/sw4/smb/MIRI-GIT/MiriTE/apt_parser/mirisim/run_sim.py", line 11, in
from mirisim import MiriSimulation
ImportError: No module named 'mirisim'

The problem is that mirisim/init.py is importing everything from run_sim.py, and run_sim.py includes the lines

  from mirisim import MiriSimulation
  from mirisim import config_parser

which attempts to reference mirisim during initialisation. There is no guarantee that the mirisim package will be available at the time the MiriTE package is built and tested. Since mirisim depends on MiriTE it is very likely MiriTE will be built and tested first. Either you need to trap the mirisim import and prevent it throwing an exception, or you need to ensure no reference is made to mirisim from any of the init.py or unit test file.

[ccossou]

I've finally understood why the tests failed. I had a function named read_test_xml in utils.py, and this function was imported in the tests (used to read a fake xml and mimic the real process).

It turns out that nosetests was thinking the whole time that this function was a test, just because there is "test" in the middle of it. Pytest was correctly ignoring this function, explaining why there was a difference between the two.

I've successfully ran nosetests on my package this time:

Ran 22 tests in 0.356s

OK

I'm still working on the "mirisim" problem. But the test will be specific to nosetests running pytest. I can't make tests working for nosetests and pytest.

[ccossou]

I'm stucked on a problem I don't really understand, @smbeard if you have an idea, I could use some help on this one.

The following are tests done in an environment without mirisim, to mimick what jenkins is doing.
As the log below is showing, apt_parser/tests are running fine, apt_parser/mirisim/tests are correctly skipped. But, somehow, when running nosetests on apt_parser as a whole, there's a 23rd tests run, I don't know what file it is, and it fails because of mirisim import.

$ nosetests apt_parser/tests/
[...]
Ran 22 tests in 0.513s

OK

$ nosetests apt_parser/mirisim/tests/
[...]
Ran 2 tests in 0.161s

OK (SKIP=2)

$ nosetests apt_parser
[...]
Ran 23 tests in 0.311s

FAILED (errors=1)

I suppose an extra file is found that nosetests consider as a test, but I couldn't find another file or function containing "test", and even the line:
nosetests -v --collect-only
isn't helpfull because I got this line:

Failure: ModuleNotFoundError (No module named 'mirisim') ... ok
and I still don't know from what file it comes from.

Note that I designed my package to keep mirisim dependant functions in a subpackage that is not imported by default. Meaning that mirisim dependency must happen only when the user tell specifically:
import apt_parser.mirisim

[smbeard]

Your latest commit now fails with this error message

test_dithering.py:1: in
from parameterized import parameterized
E ImportError: No module named 'parameterized'

The "parameterized" module is no longer recognised. Last week I upgraded my MIRICLE build to test whether Bug 575 could be closed. It looks like "parameterized" has been missed from the latest MIRICLE build.

[smbeard]

When I run nosetests -v --collect-only it lists the following files within apt_parser:

apt_parser/init.py 4 0 100%
apt_parser/constants.py 6 0 100%
apt_parser/dithering.py 30 26 13%
apt_parser/library.py 149 129 13%
apt_parser/mirisim/init.py 3 0 100%
apt_parser/mirisim/constants.py 3 0 100%
apt_parser/mirisim/extract_sim.py 86 77 10%
apt_parser/mirisim/run_sim.py 55 48 13%
apt_parser/templates.py 351 306 13%
apt_parser/tests/test_dithering.py 9 8 11%
apt_parser/tests/test_library.py 22 21 5%
apt_parser/tests/test_templates.py 81 69 15%
apt_parser/tests/test_utils.py 10 7 30%
apt_parser/utils.py 90 78 13%

I wonder if your one remaining ImportError is coming from mirisim/init.py.

Addendum: git has changed _ _ init _ _.py to init.py.

[ccossou]

I wonder why nosetests is running them.

If it stays like this, I don't even know where to start and what to do with my apt_parser.mirisim sub-package.

In the end, I still need mirisim functions for my package to work and run simulations.

[ccossou]

I'm wondering if the problem is coming from the coverage part of nosetests, because he tries to import everyhing, even if not asked.

Is it possible to constrain where the tests need to be, like apt_parser/tests, and not elsewhere?

If not, I'm afraid this merge will never be possible. (or if you have a solution, I'm open to it)

[ccossou]

I've deleted the mirisim part that is now included in the branch apt_parser of MIRISim.

No merging problem anymore.

[ccossou]

Note also that I did not found a requirements.txt to ensure all my dependencies for apt_parser were covered by miri dependencies. I think about the package parameterized in particular. Steven, if there is a way to specify the list of package dependencies, could you please tell me were it is? (incidently, I also have the same question for mirisim)