bjoernbiltzinger / pyspi Goto Github PK

The pip install py-spi command fails in a fresh conda environment on my Mac with an HDF5 compilation error. Given the dependencies on things like h5py and numba that don't always have great pip support, I'd recommend adding some dependency comments to the docs. In particular, given the huge number of dependencies (many of them compiled) I think it would be useful for a lot of users to have a conda environment or command to get started fast.

For example, the following conda / mamba environment worked for me:

mamba create -n pyspi -c conda-forge python=3.9 numpy scipy ipython numba astropy matplotlib h5py pandas pytables

These aren't all the dependencies, of course, and not all necessarily need to be installed with conda, but this might be somewhere to start?

Ref: openjournals/joss-reviews#4017

I see there is continous integration and automated testing enabled for this project, and as a user I was successfully able to run the manual tests.
However, I think in the spirit of the 'automated testing by user' implied by the review criteria: https://joss.readthedocs.io/en/latest/review_criteria.html
it would be useful and more complete to have a very simple end-user runnable test script, that one can run post local installation to automatically run through the tests in the pyspi.test package.
openjournals/joss-reviews#4017

To be consistent with the JOSS requirements, I think it would be good to expand the community guidelines, with more details about how to contribute to different aspects of the code, what the expectations are for contributors, how to set up the development environment, etc.

For some tips about how to structure this, take a look at: https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors, but you'll probably even want some more than this. For example, I think it's really useful to have dev docs describing the typical workflow for developing and contributing to the code base.

Ref: openjournals/joss-reviews#4017

When I try importing following this tutorial, it fails as follows:

>>> from pyspi.utils.data_builder.time_series_builder import TimeSeriesBuilderSPI
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/homebrew/Caskroom/miniforge/base/envs/pyspi/lib/python3.9/site-packages/pyspi/utils/data_builder/__init__.py", line 1, in <module>
    from .time_series_builder import SPISWFileGRB, TimeSeriesBuilderSPI
  File "/opt/homebrew/Caskroom/miniforge/base/envs/pyspi/lib/python3.9/site-packages/pyspi/utils/data_builder/time_series_builder.py", line 16, in <module>
    from threeML.io.file_utils import sanitize_filename
  File "/opt/homebrew/Caskroom/miniforge/base/envs/pyspi/lib/python3.9/site-packages/threeML/__init__.py", line 5, in <module>
    pd.set_option("max_columns", None)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/pyspi/lib/python3.9/site-packages/pandas/_config/config.py", line 256, in __call__
    return self.__func__(*args, **kwds)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/pyspi/lib/python3.9/site-packages/pandas/_config/config.py", line 149, in _set_option
    key = _get_single_key(k, silent)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/pyspi/lib/python3.9/site-packages/pandas/_config/config.py", line 116, in _get_single_key
    raise OptionError("Pattern matched multiple keys")
pandas._config.config.OptionError: 'Pattern matched multiple keys'

It looks like this is actually an issue with threeML, so maybe @grburgess wants to comment?

Ref: openjournals/joss-reviews#4017

As mentioned by @Tohuvavohu over in #18, the docs don't seem to currently describe the workflow for running the unit tests locally. It would be good to include this, and in particular, just running pytest doesn't work without pytest-cov installed. The docs should be clear about what the expected steps are.

Ref: openjournals/joss-reviews#4017

A fresh check out of this repo is pretty huge (it clocks in at about 1.3G right now) which is painfully slow to clone. Presumably this is because of diffs to all the test files. Using this command, I found large binary files in pyspi/data, examples, and examples/testdata. None of them are particularly huge, but diffs of binary files can eat up a lot of space. I think it would be worthwhile trying to do a little spring cleaning in the git history to try to clear out some of this cruft!

Ref: openjournals/joss-reviews#4017

Hello all,
I use conda to create a clean virtual environment, and threeml v2.3.2 and astromodels v2.3.9, pyspi, are installed in this environment.

After that, I reported this error according to the example in the tutorial

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
/tmp/ipykernel_12019/1201040965.py in <module>
      5                                         grbtime,
      6                                         ebounds=ebounds,
----> 7                                         sgl_type="both",
      8                                         )

~/miniconda3/envs/threeML/lib/python3.7/site-packages/pyspi/utils/data_builder/time_series_builder.py in from_spi_grb(cls, name, det, time_of_grb, ebounds, response, sgl_type, restore_background, poly_order, verbose)
    796                                      ebounds,
    797                                      time_of_grb,
--> 798                                      sgl_type=sgl_type)
    799 
    800         event_list = EventListWithLiveTime(

~/miniconda3/envs/threeML/lib/python3.7/site-packages/pyspi/utils/data_builder/time_series_builder.py in __init__(self, det, ebounds, time_of_grb, sgl_type)
    400             self._sgl_type = sgl_type
    401         # Find the SW ID of the data file we need for this time
--> 402         self._pointing_id = find_needed_ids(self._time_of_GRB)
    403 
    404         # Get the data, either from afs or from ISDC archive

~/miniconda3/envs/threeML/lib/python3.7/site-packages/pyspi/utils/function_utils.py in find_needed_ids(time)
     90     # Path to file, which contains id information and start and stop
     91     # time
---> 92     id_file_path = os.path.join(get_path_of_internal_data_dir(),
     93                                 'id_data_time.hdf5')
     94 

~/miniconda3/envs/threeML/lib/python3.7/site-packages/pyspi/io/package_data.py in get_path_of_internal_data_dir()
     15     Get path to the external data directory (mostly to store data there)
     16     """
---> 17     file_path = os.environ['PYSPI_PACKAGE_DATA']
     18 
     19     return file_path

~/miniconda3/envs/threeML/lib/python3.7/os.py in __getitem__(self, key)
    679         except KeyError:
    680             # raise KeyError with the original key value
--> 681             raise KeyError(key) from None
    682         return self.decodevalue(value)
    683 

KeyError: 'PYSPI_PACKAGE_DATA'

Environment variables:
.zshrc contents are:

export PYSPI=/home/jisans/workspace/external/datafolder
export PYSPI
export PYSPI_PACKAGE_DATA=/home/jisans/workspace/internal/data
export PYSPI_PACKAGE_DATA

System: Linux DESKTOP-10UCA6U 5.10.102.1-microsoft-standard-WSL2 #1 SMP Wed Mar 2 00:30:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
conda env
python 3.7.12, pip 22.2.2

As part of my review for openjournals/joss-reviews#4017, I'm going to collect minor comments and suggestions here in this issue. I'll keep adding to this list as I go, and if required, I'll open separate issues if anything more major comes up.

• Related to "A statement of need" checklist item: I think that the paper clearly specifies this need, but the online documentation would be improved with some of this text included on the landing page.
• It would also be useful to add a link to the docs from the top right corner of the GitHub landing page. Currently the only link there is to 3ML and I repeatedly clicked on that when I wanted to go to the docs for PySPI.
• In this section of the installation docs, it's not clear what the /path/to/external/datafolder is for and what it should be set to. Some clarification would be useful.
• Throughout the docs, the various data products are often visualized using built in functions (e.g. .view_lightcurve for light curves), but I think it would also be useful to demonstrate how to actually access the underlying data (as a numpy array or something) which doesn't always seem clear from the examples. I'm an outsider to the field, but I know that in my research I generally want to access the underlying data, not just see the nicely formatted plots!