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

Refactor add_two_photon_series to add_photon_series and add support for OnePhotonSeries #462

Merged
merged 25 commits into from
Jun 6, 2023

Conversation

weiglszonja
Copy link
Contributor

@weiglszonja weiglszonja commented May 30, 2023

Add support for adding data from ImagingExtractor as OnePhotonSeries to NWB file

  • refactor add_two_photon_series to add_photon_series
  • refactor get_default_ophys_metadata to add Device and ImagingPlane metadata which are both used by imaging and segmentation
  • add photon_series_type to get_nwb_imaging_metadata which can be either "OnePhotonSeries" or "TwoPhotonSeries"
  • add get_default_segmentation_metadata to fill default additional metadata for segmentation

@weiglszonja weiglszonja marked this pull request as draft May 30, 2023 13:17
@weiglszonja weiglszonja self-assigned this May 30, 2023
@weiglszonja weiglszonja changed the title Refactor add_two_photon_series to add_photon_series and add support for OnePhotonSeries Refactor add_two_photon_series to add_photon_series and add support for OnePhotonSeries Jun 1, 2023
@weiglszonja weiglszonja marked this pull request as ready for review June 6, 2023 13:00
@@ -5,8 +5,7 @@ PyYAML>=5.4
scipy>=1.4.1
h5py>=2.10.0
hdmf>=3.4.7
pynwb>=1.4.0;python_version>='3.8'
pynwb>=1.4.0,<2.3.2;python_version>='3.11'
pynwb==2.3.2;python_version>='3.8'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what about numpy?

numpy>=1.22.0,<1.24;python_version<'3.11'
numpy>=1.22.0;python_version>='3.11'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do w/e you need to while attempting to loosen all versions

The numpy bounds might have been because of the same ROI test issue

@CodyCBakerPhD
Copy link
Member

@weiglszonja Thanks for getting this capability integrated!

@CodyCBakerPhD
Copy link
Member

(the OpenEphys failure is unrelated to the numpy thing; I'll re-trigger it when the other runs complete)

@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Merging #462 (bcdbcdd) into main (4524624) will increase coverage by 0.00%.
The diff coverage is 92.30%.

❗ Current head bcdbcdd differs from pull request most recent head 9ef96cf. Consider uploading reports for the commit 9ef96cf to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #462   +/-   ##
=======================================
  Coverage   92.23%   92.23%           
=======================================
  Files          93       93           
  Lines        4338     4341    +3     
=======================================
+ Hits         4001     4004    +3     
  Misses        337      337           
Flag Coverage Δ
unittests 92.23% <92.30%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/neuroconv/tools/roiextractors/__init__.py 100.00% <ø> (ø)
src/neuroconv/tools/roiextractors/roiextractors.py 96.06% <92.30%> (+0.03%) ⬆️

@CodyCBakerPhD CodyCBakerPhD merged commit 5e14e7c into main Jun 6, 2023
@CodyCBakerPhD CodyCBakerPhD deleted the add_one_photon_series branch June 6, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants