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

version and cache spec #1358

Merged
merged 1 commit into from
Feb 29, 2020

Conversation

bendichter
Copy link
Contributor

addresses #1330

Adds required version to namespace and caches spec in file. This should allow pynwb to read the file without needing to import from allensdk.

@claassistantio
Copy link

claassistantio commented Feb 10, 2020

CLA assistant check
All committers have signed the CLA.

@wbwakeman wbwakeman added braintv relates to Insitute BrainTV program neuropixels NWB Issues related to NWB files labels Feb 10, 2020
@wbwakeman wbwakeman added the external Issues submitted from external users label Feb 19, 2020
@kschelonka kschelonka changed the base branch from master to rc/1.6.0 February 24, 2020 20:04
@kschelonka kschelonka closed this Feb 24, 2020
@kschelonka kschelonka reopened this Feb 24, 2020
@codecov-io
Copy link

Codecov Report

Merging #1358 into rc/1.6.0 will decrease coverage by 0.72%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##           rc/1.6.0    #1358      +/-   ##
============================================
- Coverage     34.35%   33.63%   -0.73%     
============================================
  Files           340      340              
  Lines         33004    33003       -1     
============================================
- Hits          11340    11099     -241     
- Misses        21664    21904     +240
Impacted Files Coverage Δ
...dk/brain_observatory/ecephys/write_nwb/__main__.py 66.17% <0%> (+0.24%) ⬆️
allensdk/brain_observatory/nwb/__init__.py 57% <0%> (-17.59%) ⬇️
...avior/behavior_ophys_api/behavior_ophys_nwb_api.py 42.93% <0%> (-15.22%) ⬇️
...ory/ecephys/stimulus_analysis/drifting_gratings.py 63.84% <0%> (-13.47%) ⬇️
allensdk/ephys/ephys_extractor.py 34.65% <0%> (-12.82%) ⬇️
...n_observatory/ecephys/stimulus_analysis/flashes.py 68.08% <0%> (-9.58%) ⬇️
...bservatory/ecephys/stimulus_analysis/dot_motion.py 85.89% <0%> (-6.42%) ⬇️
allensdk/ephys/ephys_features.py 43.85% <0%> (-4.73%) ⬇️
...atory/ecephys/stimulus_analysis/static_gratings.py 66.66% <0%> (-2.95%) ⬇️
...ephys/stimulus_analysis/receptive_field_mapping.py 86.66% <0%> (-2.78%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7611d50...391459e. Read the comment docs.

@kschelonka
Copy link
Contributor

Does the update to cache_spec when writing the files require any change on our methods to read them back in?

@bendichter
Copy link
Contributor Author

@kschelonka, No the update to cache_spec when writing the files should not require any changes on your methods to read them back in.

Right now, reading these files requires an import of the extension because the extension is not cached. With the extension cached, you will be able to read the file with

from pynwb import NWBHDF5IO
nwb = NWBHDF5IO(filepath, 'r', load_namespaces=True).read()

and it should not require any other imports. You will retain the ability to read them the old way, importing the extension and using that extension to read the file without loading the namespace.

@kschelonka kschelonka marked this pull request as ready for review February 28, 2020 23:08
@kschelonka
Copy link
Contributor

I marked this as no longer a draft. Is that correct?

@bendichter
Copy link
Contributor Author

bendichter commented Feb 28, 2020 via email

Copy link
Contributor

@kschelonka kschelonka left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for your contribution.

@kschelonka kschelonka merged commit 12f52d7 into AllenInstitute:rc/1.6.0 Feb 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
braintv relates to Insitute BrainTV program external Issues submitted from external users neuropixels NWB Issues related to NWB files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants