Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: neuroconv[edf] not loading pyedflib #1116

Closed
2 tasks done
bendichter opened this issue Oct 21, 2024 · 4 comments · Fixed by #1118
Closed
2 tasks done

[Bug]: neuroconv[edf] not loading pyedflib #1116

bendichter opened this issue Oct 21, 2024 · 4 comments · Fixed by #1118
Assignees

Comments

@bendichter
Copy link
Contributor

What happened?

I don't really know why, but when I create a new environment I can't get pyedflib to install automatically

Steps to Reproduce

% conda create -n edf-test pip
% conda activate edf-test
% pip install "neuroconv[edf]" ipython
% ipython

In [1]: from datetime import datetime
   ...: from zoneinfo import ZoneInfo
   ...: from pathlib import Path
   ...: from neuroconv.datainterfaces import EDFRecordingInterface
   ...: 
   ...: file_path = f"/Users/bendichter/data/neuroconv_testing_data/ephy_testing_data/edf/edf+C.edf"
   ...: # Change the file_path to the location in your system
   ...: interface = EDFRecordingInterface(file_path=file_path, verbose=False)
   ...: 
   ...: # Extract what metadata we can from the source files
   ...: metadata = interface.get_metadata()
   ...: # For data provenance we add the time zone information to the conversion
   ...: session_start_time = datetime(2020, 1, 1, 12, 30, 0, tzinfo=ZoneInfo("US/Pacific"))
   ...: metadata["NWBFile"].update(session_start_time=session_start_time)
   ...: 
   ...: # Choose a path for saving the nwb file and run the conversion
   ...: nwbfile_path = f"test.nwb"
   ...: interface.run_conversion(nwbfile_path=nwbfile_path, metadata=metadata)

Traceback

---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
Cell In[1], line 8
      6 file_path = f"/Users/bendichter/data/neuroconv_testing_data/ephy_testing_data/edf/edf+C.edf"
      7 # Change the file_path to the location in your system
----> 8 interface = EDFRecordingInterface(file_path=file_path, verbose=False)
     10 # Extract what metadata we can from the source files
     11 metadata = interface.get_metadata()

File /opt/miniconda3/envs/edf-test/lib/python3.13/site-packages/neuroconv/datainterfaces/ecephys/edf/edfdatainterface.py:40, in EDFRecordingInterface.__init__(self, file_path, verbose, es_key)
     26 def __init__(self, file_path: FilePath, verbose: bool = True, es_key: str = "ElectricalSeries"):
     27     """
     28     Load and prepare data for EDF.
     29     Currently, only continuous EDF+ files (EDF+C) and original EDF files (EDF) are supported
   (...)
     38     es_key : str, default: "ElectricalSeries"
     39     """
---> 40     get_package(
     41         package_name="pyedflib",
     42         excluded_platforms_and_python_versions=dict(darwin=dict(arm=["3.8", "3.9"])),
     43     )
     45     super().__init__(file_path=file_path, verbose=verbose, es_key=es_key)
     46     self.edf_header = self.recording_extractor.neo_reader.edf_header

File /opt/miniconda3/envs/edf-test/lib/python3.13/site-packages/neuroconv/tools/importing.py:123, in get_package(package_name, installation_instructions, excluded_python_versions, excluded_platforms_and_python_versions)
    120     return sys.modules[package_name]
    122 if is_package_installed(package_name=package_name) is not None:
--> 123     return import_module(name=package_name)
    125 raise ModuleNotFoundError(
    126     f"\nThe required package'{package_name}' is not installed!\n"
    127     f"To install this package, please run\n\n\t{installation_instructions}\n"
    128 )

File /opt/miniconda3/envs/edf-test/lib/python3.13/importlib/__init__.py:88, in import_module(name, package)
     86             break
     87         level += 1
---> 88 return _bootstrap._gcd_import(name[level:], package, level)

File <frozen importlib._bootstrap>:1387, in _gcd_import(name, package, level)

File <frozen importlib._bootstrap>:1360, in _find_and_load(name, import_)

File <frozen importlib._bootstrap>:1324, in _find_and_load_unlocked(name, import_)

ModuleNotFoundError: No module named 'pyedflib'

Operating System

macOS

Python Executable

Conda

Python Version

3.12

Package Versions

No response

Code of Conduct

@bendichter bendichter added the bug label Oct 21, 2024
@bendichter
Copy link
Contributor Author

bendichter commented Oct 21, 2024

I can manually install pyedflib, and then it says spikeinterface is missing, so it's a problem with extra dependencies in general. If I add a print(extras_require["edf"]) in the setup.py, I get ['spikeinterface>=0.101.0\n', 'neo>=0.13.3\n', 'lxml>=4.9.4\n', 'pyedflib>=0.1.36\n'], which looks correct. Is this a problem for all data formats? Could this be a problem with moving to the pyproject.toml? If that is the case, maybe a solution would be to move all of the extra requirements to the pyproject.toml file as well?

@bendichter
Copy link
Contributor Author

bendichter commented Oct 21, 2024

If you do a local installation by navigating the a local neuroconv directory and running pip install ".[edf]" (or pip install -e ".[edf]") this installs the optional dependencies as expected. So this looks like a divergence in behavior between the pypi installation and the local installation.

@bendichter
Copy link
Contributor Author

I think the solution may be to move from the individual modality- and format-specific requirements.txt files to having all of these extra dependences expressed in the pyproject.toml file. We could create a small script based on the current setup.py file to aggregate all of these dependencies into the pyproject.toml form, then we could delete all the requirements.txt files. One annoying thing about this is I don't see any easy way to test this without releasing it, since it does not exhibit the same behavior with local installations. I think the easiest way to test this would be to leverage the https://test.pypi.org/ server

@h-mayorquin h-mayorquin self-assigned this Oct 21, 2024
@h-mayorquin
Copy link
Collaborator

If you do a local installation by navigating the a local neuroconv directory and running pip install ".[edf]" (or pip install -e ".[edf]") this installs the optional dependencies as expected. So this looks like a divergence in behavior between the pypi installation and the local installation.

Damn, and I was happy because we were testing for this but I did not expect at all that the behavior would be different between building locally and whatever we are uploading to pipy.

I think the solution may be to move from the individual modality- and format-specific requirements.txt files to having all of these extra dependences expressed in the pyproject.toml file. We could create a small script based on the current setup.py file to aggregate all of these dependencies into the pyproject.toml form, then we could delete all the requirements.txt files.

I think this is the simplest solution. I will give it a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants