[REVIEW]: ImagingReso: A Tool for Neutron Resonance Imaging

[whedon]

Submitting author: @zhangy6x (Yuxuan Zhang) Repository: https://github.com/ornlneutronimaging/ImagingReso Version: v1.5.8 Editor: @arokem Reviewer: @nicoguaro Archive: 10.5281/zenodo.1054038

Status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/997d09281a9d76e95f4ec4d3279eeb8c"><img src="http://joss.theoj.org/papers/997d09281a9d76e95f4ec4d3279eeb8c/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/997d09281a9d76e95f4ec4d3279eeb8c/status.svg)](http://joss.theoj.org/papers/997d09281a9d76e95f4ec4d3279eeb8c)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer questions

@nicoguaro, please carry out your review in this issue by updating the checklist below (please make sure you're logged in to GitHub). The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @arokem know.

Conflict of interest

• [x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• [x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• [x] Repository: Is the source code for this software available at the repository url?
• [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• [x] Version: Does the release version given match the GitHub release (v1.5.8)?
• [x] Authorship: Has the submitting author (@zhangy6x) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

• [x] Installation: Does installation proceed as outlined in the documentation?
• [x] Functionality: Have the functional claims of the software been confirmed?
• [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• [x] Authors: Does the paper.md file include a list of authors with their affiliations?
• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

[whedon]

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @nicoguaro it looks like you're currently assigned as the reviewer for this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As as reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all JOSS reviews 😿

To fix this do the following two things:

1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

2. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

For a list of things I can do to help you, just type:

@whedon commands

[nicoguaro]

@zhangy6x, I suggest that you make explicit the dependence on Python 3. I will need to repeat the process since I created an environment for Python 2.7 instead.

[zhangy6x]

@nicoguaro: Good point, I just addressed the Python version supported in the README.rst.

[arokem]

@nicoguaro : have you had a chance to take a look at the other items on the check list above?

[nicoguaro]

Yes, I installed the software for Python 3 as described in the instructions.

I also try to run the tests using pytest and this is the errors I get

    =========================================== ERRORS ===========================================
    ____________________ ERROR collecting tests/ImagingReso/utilities_test.py ____________________
    ImportError while importing test module '/home/nicoguaro/workspace/ImagingReso/tests/ImagingReso/utilities_test.py'.
    Hint: make sure your test modules/packages have valid Python names.
    Traceback:
    utilities_test.py:7: in <module>
        from ImagingReso.resonance import Resonance
    E   ImportError: No module named ImagingReso.resonance
    !!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!

and

=========================================== ERRORS ===========================================
____________________ ERROR collecting tests/ImagingReso/resonance_test.py ____________________
ImportError while importing test module '/home/nicoguaro/workspace/ImagingReso/tests/ImagingReso/resonance_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
resonance_test.py:4: in <module>
    from ImagingReso.resonance import Resonance
E   ImportError: No module named ImagingReso.resonance
!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!
================================== 1 error in 0.23 seconds ===================================

[zhangy6x]

@nicoguaro I was able to reproduce the error. Look like it is the path issues with the pytest. Could you please try to runpython -m pytest tests/ in the repo dir. It fixes this problem on my end.

[zhangy6x]

@nicoguaro Were you able to run the unit test with python -m pytest tests/?

[nicoguaro]

I was not. I obtain the following error

_____________ ERROR collecting tests/ImagingReso/resonance_test.py _____________
../../anaconda2/lib/python2.7/site-packages/_pytest/python.py:418: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
../../anaconda2/lib/python2.7/site-packages/py/_path/local.py:662: in pyimport
    __import__(modname)
../../anaconda2/lib/python2.7/site-packages/_pytest/assertion/rewrite.py:216: in load_module
    py.builtin.exec_(co, mod.__dict__)
tests/ImagingReso/resonance_test.py:4: in <module>
    from ImagingReso.resonance import Resonance
E     File "ImagingReso/resonance.py", line 115
E       self.stack = {**self.stack, **new_stack}
E                      ^
E   SyntaxError: invalid syntax

[zhangy6x]

@nicoguaro I was able to reproduce your error. Could you please try to run it again under Python 3.6 instead of 2.7? If you would like, you could also find the test results on Travis (https://travis-ci.org/ornlneutronimaging/ImagingReso). The tutorial.ipynb in notebook dir can also be used to test all functionalities provided. Thanks.

[nicoguaro]

@zhangy6x, I started from scratch. I removed the virtual environment and then installed it once again from the very beginning, this time using Python 3.6.

This is the result of the test (python -m pytest tests/):

============================= test session starts ==============================
platform linux -- Python 3.6.2, pytest-3.2.1, py-1.4.34, pluggy-0.4.0
rootdir: /home/nguarinz/workspace/ImagingReso, inifile:
collected 66 items

tests/ImagingReso/resonance_test.py ..F...............................
tests/ImagingReso/utilities_test.py .......F........................

=================================== FAILURES ===================================
_____________________ TestInitialization.test_adding_layer _____________________

self = <resonance_test.TestInitialization testMethod=test_adding_layer>

    def test_adding_layer(self):
        """assert adding_layer works"""
        o_reso = Resonance()

        # layer 1
        layer1 = 'CoAg'
        thickness1 = 0.025
        o_reso.add_layer(formula=layer1, thickness=thickness1)

        # layer 2
        layer2 = 'Ag'
        thickness2 = 0.1
        density2 = 0.5
        o_reso.add_layer(formula=layer2, thickness=thickness2, density=density2)

        returned_stack = o_reso.stack

        expected_stack = {'CoAg': {'elements': ['Co', 'Ag'],
                                   'stoichiometric_ratio': [1, 1],
                                   'thickness': {'value': 0.025,
                                                 'units': 'mm'},
                                   'density': {'value': 9.7,
                                               'units': 'g/cm3'},
                                   'Co': {'isotopes': {'file_names': ['Co-58.csv', 'Co-59.csv'],
                                                       'list': ['58-Co', '59-Co'],
                                                       'mass': {'value': [57.9357576, 58.9332002],
                                                                'units': 'g/mol',
                                                                },
                                                       'isotopic_ratio': [0.0, 1.0],
                                                       },
                                          'density': {'value': 8.9,
                                                      'units': 'g/cm3'},
                                          'molar_mass': {'value': 58.9332,
                                                         'units': 'g/mol'},
                                          },
                                   'Ag': {'isotopes': {'file_names': ['Ag-107.csv', 'Ag-109.csv'],
                                                       'list': ['107-Ag', '109-Ag'],
                                                       'mass': {'value': [106.905093, 108.904756],
                                                                'units': 'g/mol',
                                                                },
                                                       'isotopic_ratio': [0.51839, 0.48161000],
                                                       },
                                          'density': {'value': 10.5,
                                                      'units': 'g/cm3'},
                                          'molar_mass': {'value': 107.8682,
                                                         'units': 'g/cm3'},
                                          },
                                   },
                          'Ag': {'elements': ['Ag'],
                                 'stoichiometric_ratio': [1],
                                 'thickness': {'value': 0.1,
                                               'units': 'mm'},
                                 'density': {'value': 0.5,
                                             'units': 'g/cm3'},
                                 'Ag': {'isotopes': {'file_names': ['Ag-107.csv', 'Ag-109.csv'],
                                                     'list': ['107-Ag', '109-Ag'],
                                                     'mass': {'value': [106.905093, 108.904756],
                                                              'units': 'g/mol',
                                                              },
                                                     'isotopic_ratio': [0.51839, 0.48161000],
                                                     },
                                        'density': {'value': 10.5,
                                                    'units': 'g/cm3'},
                                        'molar_mass': {'value': 107.8682,
                                                       'units': 'g/mol'},
                                        },
                                 },
                          }

        # CoAg
        # elements
        self.assertEqual(returned_stack['CoAg']['elements'], expected_stack['CoAg']['elements'])
        # atomic_atomic_ratio
        self.assertEqual(returned_stack['CoAg']['stoichiometric_ratio'], expected_stack['CoAg']['stoichiometric_ratio'])
        # thickness
        self.assertEqual(returned_stack['CoAg']['thickness']['value'], expected_stack['CoAg']['thickness']['value'])
        self.assertEqual(returned_stack['CoAg']['thickness']['units'], expected_stack['CoAg']['thickness']['units'])
        # isotopes Co
        # file names
        self.assertEqual(returned_stack['CoAg']['Co']['isotopes']['file_names'],
>                        expected_stack['CoAg']['Co']['isotopes']['file_names'])
E       AssertionError: Lists differ: ['Co-59.csv', 'Co-58.csv'] != ['Co-58.csv', 'Co-59.csv']
E       
E       First differing element 0:
E       'Co-59.csv'
E       'Co-58.csv'
E       
E       - ['Co-59.csv', 'Co-58.csv']
E       + ['Co-58.csv', 'Co-59.csv']

tests/ImagingReso/resonance_test.py:243: AssertionError
______ TestUtilities_1.test_get_isotope_dicts_returns_correct_dictionary _______

self = <utilities_test.TestUtilities_1 testMethod=test_get_isotope_dicts_returns_correct_dictionary>

    def test_get_isotope_dicts_returns_correct_dictionary(self):
        '''assert get_isotope_dict works with typical entry element Ag'''
        _element = 'Ag'
        _dict_returned = get_isotope_dicts(element=_element)
        _dict_expected = {'density': {'value': 10.5,
                                      'units': 'g/cm3'},
                          'isotopes': {'list': ['107-Ag','109-Ag','110-Ag', '111-Ag'],
                                       'file_names': ['Ag-107.csv','Ag-109.csv','Ag-110.csv','Ag-111.csv'],
                                       'mass': {'value': [106.905093,108.904756,109.90611,110.905295],
                                                'units': 'g/mol'},
                                       'density': {'value': [10.406, 10.600, 10.698, 10.795],
                                                   'units': 'g/cm3'},
                                       'isotopic_ratio': [0.51839, 0.481610, 0.0, 0.0]},
                          'molar_mass': {'value': 107.8682,
                                         'units': 'g/cm3'},
                          }

        # list of isotopes
>       self.assertEqual(_dict_returned['isotopes']['list'], _dict_expected['isotopes']['list'])
E       AssertionError: Lists differ: ['111-Ag', '110-Ag', '107-Ag', '109-Ag'] != ['107-Ag', '109-Ag', '110-Ag', '111-Ag']
E       
E       First differing element 0:
E       '111-Ag'
E       '107-Ag'
E       
E       - ['111-Ag', '110-Ag', '107-Ag', '109-Ag']
E       + ['107-Ag', '109-Ag', '110-Ag', '111-Ag']

tests/ImagingReso/utilities_test.py:179: AssertionError
===================== 2 failed, 64 passed in 7.91 seconds ======================

[zhangy6x]

@nicoguaro I wasn't able to reproduce this error. After reading the log, ['Co-59.csv', 'Co-58.csv'] != ['Co-58.csv', 'Co-59.csv'] and ['111-Ag', '110-Ag', '107-Ag', '109-Ag'] != ['107-Ag', '109-Ag', '110-Ag', '111-Ag'], it looks like that the tests failed due to the descending sequence of the elements arranged in the lists returned. I noticed that you are using a Linux machine, so I added Linux to my Travis setting and it passed all the tests as shown below. I am not sure why you got the error you after runing the tests. Maybe the other author, @JeanBilheux, could help us to investigate this.

platform linux -- Python 3.6.2, pytest-3.2.3, py-1.4.34, pluggy-0.4.0 -- /home/travis/mc/envs/testenv/bin/python
cachedir: .cache
rootdir: /home/travis/build/ornlneutronimaging/ImagingReso, inifile:
plugins: cov-2.5.1
collected 61 items                                                              
tests/ImagingReso/resonance_test.py::TestInitialization::test_E_max PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_E_min PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_adding_layer PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_correct_initialization_of_stack PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_element_metadata_via_add_layer_initialization PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_element_metadata_via_stack_initialization PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_get_sigma_isotopes PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_initialization_E_step_bigger_than_E_range_raises_error PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_initialization_of_object PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_initialization_with_same_E_min_and_max_raises_error PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_layer_density_locked_if_defined_during_initialization_add_layer PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_layer_density_locked_if_defined_during_initialization_init PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_repr PASSED
tests/ImagingReso/resonance_test.py::TestInitialization::test_str PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_density_returned PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_density_returned_all PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_if_element_misssing_use_compound_and_raises_error_if_wrong PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_if_element_misssing_use_compound_and_raises_error_if_wrong_when_getting_density PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_list_all_stoichiometric_ratio PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_retrieve_density_raises_error_if_unknown_compound PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_retrieve_stoichiometric_of_UO3_sample PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_retrieve_stoichiometric_ratio_raises_error_if_unknown_compound PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_set_stoichiometric_ratio_correctly_calculates_new_density PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_set_stoichiometric_ratio_correctly_calculates_new_molar_mass PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_set_stoichiometric_ratio_has_correct_new_list_size PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_set_stoichiometric_ratio_raises_value_error_if_wrong_compound PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_set_stoichiometric_ratio_raises_value_error_if_wrong_element PASSED
tests/ImagingReso/resonance_test.py::TestGetterSetter::test_stoichiometric_ratio_returned PASSED
tests/ImagingReso/resonance_test.py::TestTransmissionAttenuation::test_calculate_attenuation_compound PASSED
tests/ImagingReso/resonance_test.py::TestTransmissionAttenuation::test_calculate_attenuation_element PASSED
tests/ImagingReso/resonance_test.py::TestTransmissionAttenuation::test_calculate_attenuation_isotopes PASSED
tests/ImagingReso/resonance_test.py::TestTransmissionAttenuation::test_calculate_transmission_compound PASSED
tests/ImagingReso/resonance_test.py::TestTransmissionAttenuation::test_calculate_transmission_element PASSED
tests/ImagingReso/resonance_test.py::TestTransmissionAttenuation::test_calculate_transmission_isotopes PASSED
tests/ImagingReso/resonance_test.py::TestPlot::test_axis_type_and_time_unit PASSED
tests/ImagingReso/resonance_test.py::TestExport::test_axis_type_and_time_unit PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_checking_stack PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_checking_stack_raises_value_error_if_element_missing_from_database PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_checking_stack_raises_value_error_if_thickness_has_wrong_format PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_formula_to_dictionary_raises_error_when_unknow_element PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_formula_to_dictionary_works_with_various_cases PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_get_compound_density PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_get_density PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_get_isotope_dicts_returns_correct_dictionary PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_get_isotope_dicts_returns_correct_element PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_get_isotope_returns_empty_dict_if_missing_element PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_get_list_of_element_raise_error_if_wrong_database PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_get_mass PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_is_element_in_database PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_1::test_raises_error_when_size_stoichiometric_ratio_is_different_from_elements PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_angstroms_s_conversion PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_calculate_transmission PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_energy_angstroms_conversion PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_ev_s_conversion PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_ev_to_image_number_conversion PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_get_atoms_per_cm3_of_layer PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_get_database_name_raises_error_if_wrong_file PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_get_database_works PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_get_interpolated_data PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_get_sigma PASSED
tests/ImagingReso/utilities_test.py::TestUtilities_2::test_set_distance_units PASSED
----------- coverage: platform linux, python 3.6.2-final-0 -----------
Name                                  Stmts   Miss  Cover
---------------------------------------------------------
tests/ImagingReso/resonance_test.py     324      0   100%
tests/ImagingReso/utilities_test.py     216      0   100%
---------------------------------------------------------
TOTAL                                   540      0   100%
========================== 61 passed in 12.24 seconds ==========================```

[nicoguaro]

@zhangy6x, I also tried running the notebook in the repo, and obtained this:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-25-7b2e6a77814b> in <module>()
----> 1 energy_UO3_U_238_angstroms = o_reso.convert_x_axis(array=energy_UO3_U_238U, from_units='ev', to_units='angstroms')

AttributeError: 'Resonance' object has no attribute 'convert_x_axis'

[zhangy6x]

@nicoguaro I am sorry, a while after submitting this code, function o_reso.convert_x_axis has been replaced by another keywords argument (x_axis=, 'energy' or 'lambda' or 'time' or 'number'. Type of x-axis.) in o_reso.plot() function. Could you please install the latest version of ImagingReso and use the notebook from the current repo to test? Sorry for any inconveniences.

[nicoguaro]

I reinstalled ImagingReso and recloned the repo.

This is the test report:

============================= test session starts ==============================
platform linux -- Python 3.6.3, pytest-3.2.1, py-1.4.34, pluggy-0.4.0
rootdir: /home/nguarinz/workspace/ImagingReso, inifile:
collected 61 items

tests/ImagingReso/resonance_test.py ..F.................................
tests/ImagingReso/utilities_test.py .......F.................

=================================== FAILURES ===================================
_____________________ TestInitialization.test_adding_layer _____________________

self = <resonance_test.TestInitialization testMethod=test_adding_layer>

    def test_adding_layer(self):
        """assert adding_layer works"""
        o_reso = Resonance()

        # layer 1
        layer1 = 'CoAg'
        thickness1 = 0.025
        o_reso.add_layer(formula=layer1, thickness=thickness1)

        # layer 2
        layer2 = 'Ag'
        thickness2 = 0.1
        density2 = 0.5
        o_reso.add_layer(formula=layer2, thickness=thickness2, density=density2)

        returned_stack = o_reso.stack

        expected_stack = {'CoAg': {'elements': ['Co', 'Ag'],
                                   'stoichiometric_ratio': [1, 1],
                                   'thickness': {'value': 0.025,
                                                 'units': 'mm'},
                                   'density': {'value': 9.7,
                                               'units': 'g/cm3'},
                                   'Co': {'isotopes': {'file_names': ['Co-58.csv', 'Co-59.csv'],
                                                       'list': ['58-Co', '59-Co'],
                                                       'mass': {'value': [57.9357576, 58.9332002],
                                                                'units': 'g/mol',
                                                                },
                                                       'isotopic_ratio': [0.0, 1.0],
                                                       },
                                          'density': {'value': 8.9,
                                                      'units': 'g/cm3'},
                                          'molar_mass': {'value': 58.9332,
                                                         'units': 'g/mol'},
                                          },
                                   'Ag': {'isotopes': {'file_names': ['Ag-107.csv', 'Ag-109.csv'],
                                                       'list': ['107-Ag', '109-Ag'],
                                                       'mass': {'value': [106.905093, 108.904756],
                                                                'units': 'g/mol',
                                                                },
                                                       'isotopic_ratio': [0.51839, 0.48161000],
                                                       },
                                          'density': {'value': 10.5,
                                                      'units': 'g/cm3'},
                                          'molar_mass': {'value': 107.8682,
                                                         'units': 'g/cm3'},
                                          },
                                   },
                          'Ag': {'elements': ['Ag'],
                                 'stoichiometric_ratio': [1],
                                 'thickness': {'value': 0.1,
                                               'units': 'mm'},
                                 'density': {'value': 0.5,
                                             'units': 'g/cm3'},
                                 'Ag': {'isotopes': {'file_names': ['Ag-107.csv', 'Ag-109.csv'],
                                                     'list': ['107-Ag', '109-Ag'],
                                                     'mass': {'value': [106.905093, 108.904756],
                                                              'units': 'g/mol',
                                                              },
                                                     'isotopic_ratio': [0.51839, 0.48161000],
                                                     },
                                        'density': {'value': 10.5,
                                                    'units': 'g/cm3'},
                                        'molar_mass': {'value': 107.8682,
                                                       'units': 'g/mol'},
                                        },
                                 },
                          }

        # CoAg
        # elements
        self.assertEqual(returned_stack['CoAg']['elements'], expected_stack['CoAg']['elements'])
        # atomic_atomic_ratio
        self.assertEqual(returned_stack['CoAg']['stoichiometric_ratio'], expected_stack['CoAg']['stoichiometric_ratio'])
        # thickness
        self.assertEqual(returned_stack['CoAg']['thickness']['value'], expected_stack['CoAg']['thickness']['value'])
        self.assertEqual(returned_stack['CoAg']['thickness']['units'], expected_stack['CoAg']['thickness']['units'])
        # isotopes Co
        # file names
        self.assertEqual(returned_stack['CoAg']['Co']['isotopes']['file_names'],
>                        expected_stack['CoAg']['Co']['isotopes']['file_names'])
E       AssertionError: Lists differ: ['Co-59.csv', 'Co-58.csv'] != ['Co-58.csv', 'Co-59.csv']
E       
E       First differing element 0:
E       'Co-59.csv'
E       'Co-58.csv'
E       
E       - ['Co-59.csv', 'Co-58.csv']
E       + ['Co-58.csv', 'Co-59.csv']

tests/ImagingReso/resonance_test.py:243: AssertionError
______ TestUtilities_1.test_get_isotope_dicts_returns_correct_dictionary _______

self = <utilities_test.TestUtilities_1 testMethod=test_get_isotope_dicts_returns_correct_dictionary>

    def test_get_isotope_dicts_returns_correct_dictionary(self):
        """assert get_isotope_dict works with typical entry element Ag"""
        _element = 'Ag'
        _dict_returned = get_isotope_dicts(element=_element)
        _dict_expected = {'density': {'value': 10.5,
                                      'units': 'g/cm3'},
                          'isotopes': {'list': ['107-Ag', '109-Ag', '110-Ag', '111-Ag'],
                                       'file_names': ['Ag-107.csv', 'Ag-109.csv', 'Ag-110.csv', 'Ag-111.csv'],
                                       'mass': {'value': [106.905093, 108.904756, 109.90611, 110.905295],
                                                'units': 'g/mol'},
                                       'density': {'value': [10.406, 10.600, 10.698, 10.795],
                                                   'units': 'g/cm3'},
                                       'isotopic_ratio': [0.51839, 0.481610, 0.0, 0.0]},
                          'molar_mass': {'value': 107.8682,
                                         'units': 'g/cm3'},
                          }

        # list of isotopes
>       self.assertEqual(_dict_returned['isotopes']['list'], _dict_expected['isotopes']['list'])
E       AssertionError: Lists differ: ['111-Ag', '110-Ag', '107-Ag', '109-Ag'] != ['107-Ag', '109-Ag', '110-Ag', '111-Ag']
E       
E       First differing element 0:
E       '111-Ag'
E       '107-Ag'
E       
E       - ['111-Ag', '110-Ag', '107-Ag', '109-Ag']
E       + ['107-Ag', '109-Ag', '110-Ag', '111-Ag']

tests/ImagingReso/utilities_test.py:178: AssertionError
===================== 2 failed, 59 passed in 8.16 seconds ======================

[zhangy6x]

@nicoguaro Thanks a lot for reinstalling everything! This test error is same as the previous issue of descending arrangement of the elements in the lists returned ['Co-59.csv', 'Co-58.csv'] != ['Co-58.csv', 'Co-59.csv'] and ['111-Ag', '110-Ag', '107-Ag', '109-Ag'] != ['107-Ag', '109-Ag', '110-Ag', '111-Ag']. This test works okay on my osx machine and linux on travis. I don't know why it failed. @JeanBilheux, could you please help me with this? Thanks.

[JeanBilheux]

@nicoguaro I updated the code to force the sorting of the Isotopes files. This should allow the unit test to work on any platform.

[nicoguaro]

@JeanBilheux, should I reinstall then?

pip install --upgrade ImagingReso

would make the trick?

[JeanBilheux]

it will in 10minutes... just the time to update the pypi library.

[nicoguaro]

I needed to git pull the repository, but got the tests to pass. Below is the output

============================= test session starts ==============================
platform linux -- Python 3.6.3, pytest-3.2.1, py-1.4.34, pluggy-0.4.0
rootdir: /home/nguarinz/workspace/ImagingReso, inifile:
collected 61 items

tests/ImagingReso/resonance_test.py ....................................
tests/ImagingReso/utilities_test.py .........................

========================== 61 passed in 7.99 seconds ===========================

[nicoguaro]

@zhangy6x, the tests are working now in my machine. I will add details of the review partially. Below are some comments:

• The project presents a statement of need of the software. Nevertheless, I think that it can be further improved for a wider audience.
• The project includes data in the repository. As far as I understand, the database is in the public domain, but I would like the authors to confirm this point.
• There are not community guidelines in the repository.
• I ran the IPython notebook and it worked this time. A little detail at the very end is the following when I try to export data as instructed in the previous cell.

with output:

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
<ipython-input-43-b67aeaf1b993> in <module>()
      1 o_reso.export(y_axis='attenuation', items_to_export=[['CoAg', 'Co'],['CoAg','Ag','107-Ag']],
----> 2              filename="resonance.csv", to_csv=True)

~/anaconda2/envs/ImagingReso/lib/python3.6/site-packages/ImagingReso/resonance.py in export(self, filename, to_csv, x_axis, y_axis, mixed, all_layers, all_elements, all_isotopes, items_to_export, time_unit, offset_us, time_resolution_us, source_to_detector_m, t_start_us)
    769                     while _path_to_export:
    770                         _item = _path_to_export.pop(0)
--> 771                         _live_path = _live_path[_item]
    772                     _y_axis = _live_path[y_axis_tag]
    773                     df[_label] = _y_axis

KeyError: 'CoAg'

Also, it is not clear to me why do you have to tell the method export explicitly that you want to write the export file. Is there a reason behind this?

@arokem, is it OK to make suggestions regarding things like PEP8?

[zhangy6x]

@nicoguaro Thanks a lot for your comments.

• We are very happy that you find this package could be further improved for a wider audience. When we started writing this package, it was mainly for simulating the resonance signal expected from neutron imaging measurement. Of course, we are happy to further improve it in later versions as you suggested.
• Yes, we did receive the positive reply from the NNDC database owner, David Brown (email: dbrown@bnl.gov, Brookhaven National Laboratory), of including the database in our package. And we did cite related ENDF publication from his group.
• I didn't notice that the community guidelines are required for this submission. I am happy to add them into README.rst once I discussed it with @JeanBilheux.
• I am sorry, I just updated the tutorial and now it is showing the correct instruction regarding exporting specified items. The new version will be available for pip install once Jean released it on PyPi tomorrow.
• The export function here is mainly for providing options if the user would like to construct their own plot with the signal obtained from this package. Since the resulted resonance signal is fairly complex, instead of exporting the whole thing, the user can choose different types of the x-axis (energy or wavelength of neutrons) and the y-axis (attenuation, transmission or cross-sections) to make their own graph conveniently.

[JeanBilheux]

@nicoguaro

• Can you be more specific about "improve for a wider audience"? Do you mean X_ray community? other? What kind of improvements do you think about?
• Same thing for the "community guidelines", what is this?

[nicoguaro]

@JeanBilheux

• I can try, at least. With wider audience I mean ... wider. I don't think that the software will be used by a person with zero knowledge of the topic. But if the description is good enough for any person to understand what is does, it does no harm. Of course, It is really difficult to write something that "any person" can understand, but I would bet for something broader than a specific community.
• Community guidelines refer to explicitly state how to contribute to the project. The importance of this is based on the openness of the software. Check the author guidelines, for further detail.

[zhangy6x]

@nicoguaro Community guidelines have been added to README. Thanks.

[nicoguaro]

@arokem, any guidelines regarding formatting?

[arokem]

@nicoguaro : do you mean the formatting of the paper? Or the community guidelines?

Also, @zhangy6x : I noticed that you have added a notice of copyright to the paper. Could you please explain what it means, and why you need it? We usually license these articles under a Creative Commons license, and this might be in conflict (?)

[arokem]

In particular, JOSS does not claim any copyright, and the work published is licensed under CC-BY. That means that anyone can reproduce the work (under the CC-BY license), and you can especially do so because you retain copyright on the work.

So - would you mind removing that notice of copyright please?

Other than that - I think that the paper is formatted as needed

[nicoguaro]

@arokem, no, I was referring to the code. Is it okay to suggest to make some changes regarding formatting?

[JeanBilheux]

@arokem Our program is using some data from the Brookhaven database (listed in the md file). Those are available publicly but to be sure we contacted one of the people in charge of the database. He told us that we were allowed to use the data as long as we were acknowledging them for the data. In the future, we are planning on moving this code into the scikit-beam project, and this is the reason we matched our license with their package.

[arokem]

@JeanBilheux : You can certainly acknowledge the use of the Brookhaven database (and I see that you are referring to it in the text of your paper)! But I was referring to the section before the acknowledgements (titled "notice of copyright"). You cannot set the copyright on the paper in this way.

[arokem]

@nicoguaro : you can certainly make comments about the formatting of the code, but the authors might not be required to comply with your comments as a requirement for publication. We do not require that code be formatted in a particular way as a condition for publication.

[nicoguaro]

@arokem, I understand that they are not required to comply. But it was better to ask before suggesting, I think.

@zhangy6x, I suggest to check the PEP8 guidelines for style in Python in the sourcecode.

[JeanBilheux]

@arokem Sorry, I got it now. I'm currently checking this with my management and our lawyers here at the lab (to play it safe). I will get back to you soon.

[JeanBilheux]

@arokem Here is the reply I got from the management/lawyers.

""" ORNL’s prime contract with DOE requires the copyright in acknowledgement statements. The notice is a Prime Contract requirement. Specifically, Paragraph I.141(d)(2) of our Prime Contract states: “The contractor shall mark each scientific or technical article first produced or composed under this Contract and submitted for journal publication or similar means of dissemination with a notice, similar in all material respects to the following, on the front reflecting the Government's non-exclusive, paid-up, irrevocable, world-wide license in the copyright.”

I verified with ORNL’s IP counsel that the CC-BY license is not in conflict with the government’s retained rights in the publication.

I would suggest that you respond that, “Our prime contract with DOE requires this statement. In addition, the CC-BY license is not in conflict with the government’s retained rights in the publication.” """

[arokem]

After some conversation with other JOSS editors, I am starting to understand that the copyright notice certainly is not in conflict with the CC-BY license (I was wrong in stating this before), but it is also really not necessary, and your paper will be less confusing to readers if you remove it. This is because, as the contract you cite from states, you are required to use a notice that is "similar in all material respects to the following". The CC-BY license is already that - anyone can reproduce the article, and certainly the United States government (or anyone designated by US government) can do so. I believe that readers will be un-neccesarily confused by this notice (as was I).

[zhangy6x]

@arokem Thanks a lot for your help throughout this review process. Please excuse us for this belated response since I don't have much knowledge about copyright issues. After discussion, we are glad that this copyright notice is not conflicting with the CC-BY license, and we think it is better to keep it based on the Prime Contract requirement. However, we could move this notice to a more appropriate place to avoid any confusion. If that is the case, please advise us where should this notice be kept. Thanks much!

[zhangy6x]

@nicoguaro PEP8 check and formatting has been performed as you suggested. Thanks.

[nicoguaro]

@arokem, I consider that this review is ready to go.

[arokem]

@zhangy6x : how about moving this from its own section to the "acknowledgements" section?

[zhangy6x]

@arokem : Notice of Copyright has been moved to "acknowledgments" section. Thanks!

[arokem]

OK. Please create an archive of the software/paper (e.g., using Zenodo) in its current state and post the DOI here.

[zhangy6x]

The DOI is 10.5281/zenodo.1052005

[zhangy6x]

https://zenodo.org/badge/latestdoi/97753107

[arokem]

@whedon set 10.5281/zenodo.1052005 as archive

[whedon]

OK. 10.5281/zenodo.1052005 is the archive.

[arokem]

@arfon: I believe this paper is ready for publication

[arfon]

@zhangy6x - Could you make sure you cite the references you currently have in the file paper.bib file in the markdown paper (paper.md) (You can read how to do that here)

[zhangy6x]

@arfon : I have updated paper.md and created a new archive on Zenodo (DOI: 10.5281/zenodo.1054038). Thanks much.

[arfon]

@whedon set 10.5281/zenodo.1054038 as archive

[whedon]

OK. 10.5281/zenodo.1054038 is the archive.