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

140 multiple outputs #311

Merged
merged 17 commits into from
Jul 21, 2022
Merged

140 multiple outputs #311

merged 17 commits into from
Jul 21, 2022

Conversation

dfsp-spirit
Copy link
Collaborator

@dfsp-spirit dfsp-spirit commented Jul 20, 2022

Note: Despite the name, this does NOT implement multiple outputs for compute functions. It is only a step in that direction, and removes the hdr/memmap support.

Author Guidelines

  • Is the change set < 600 lines?
  • Was the code checked for memory leaks/performance bottlenecks?
  • Is the code running locally and on the ESI cluster?
  • Is the code running on all supported platforms?

Reviewer Checklist

  • Are testing routines present?
  • Do parallel loops have a set length and correct termination conditions?
  • Do objects in the global package namespace perform proper parsing of their input?
  • Do code-blocks provide novel functionality, i.e., no re-factoring using builtin/external packages possible?
  • Code layout
    • Is the code PEP8 compliant?
    • Does the code adhere to agreed-upon naming conventions?
    • Are keywords clearly named and easy to understand?
    • No commented-out code?
  • Are all docstrings complete and accurate?
  • Is the CHANGELOG.md up to date?

tensionhead and others added 15 commits June 24, 2022 15:39
- first ideas how to attach additional data to the hdf5

On branch 140-multiple-outputs
Changes to be committed:
	modified:   syncopy/shared/kwarg_decorators.py
On branch 140-multiple-outputs
Changes to be committed:
	modified:   syncopy/shared/kwarg_decorators.py
- all h5py reading and writing should be done within this decorator
- already the case for parallel processing
- TODO: move sequential part also into this decorator

Changes to be committed:
	modified:   syncopy/datatype/methods/arithmetic.py
	modified:   syncopy/datatype/methods/padding.py
	modified:   syncopy/datatype/methods/selectdata.py
	modified:   syncopy/nwanalysis/AV_compRoutines.py
	modified:   syncopy/nwanalysis/ST_compRoutines.py
	modified:   syncopy/preproc/compRoutines.py
	modified:   syncopy/shared/computational_routine.py
	modified:   syncopy/shared/kwarg_decorators.py
	modified:   syncopy/specest/compRoutines.py
	modified:   syncopy/tests/test_computationalroutine.py
- removed the comment-like return value

Changes to be committed:
	modified:   syncopy/shared/kwarg_decorators.py
Changes to be committed:
	modified:   syncopy/specest/compRoutines.py
	modified:   syncopy/tests/local_spy.py
- now it's a pretty line break

Changes to be committed:
	modified:   syncopy/shared/computational_routine.py
@tensionhead
Copy link
Contributor

As the removed memmap support never was exposed to the user (afaik), I think we don't need any changelog entry

tensionhead and others added 2 commits July 20, 2022 16:46
Changes to be committed:
	modified:   syncopy/io/save_spy_container.py
@codecov
Copy link

codecov bot commented Jul 21, 2022

Codecov Report

Merging #311 (a37611c) into dev (730eda5) will increase coverage by 0.06%.
The diff coverage is 78.94%.

@@            Coverage Diff             @@
##              dev     #311      +/-   ##
==========================================
+ Coverage   72.36%   72.42%   +0.06%     
==========================================
  Files          68       68              
  Lines        7904     7704     -200     
  Branches     1629     1581      -48     
==========================================
- Hits         5720     5580     -140     
+ Misses       1787     1744      -43     
+ Partials      397      380      -17     
Impacted Files Coverage Δ
syncopy/io/_load_nwb.py 10.00% <ø> (ø)
syncopy/io/load_spy_container.py 73.60% <ø> (+1.50%) ⬆️
syncopy/datatype/base_data.py 82.43% <47.36%> (-0.79%) ⬇️
syncopy/datatype/statistical_data.py 39.62% <60.00%> (-0.57%) ⬇️
syncopy/shared/kwarg_decorators.py 84.18% <66.66%> (+2.70%) ⬆️
syncopy/datatype/continuous_data.py 85.87% <100.00%> (-0.17%) ⬇️
syncopy/datatype/discrete_data.py 83.33% <100.00%> (-0.31%) ⬇️
syncopy/datatype/methods/arithmetic.py 78.45% <100.00%> (ø)
syncopy/datatype/methods/padding.py 70.38% <100.00%> (ø)
syncopy/datatype/methods/selectdata.py 78.75% <100.00%> (ø)
... and 11 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 730eda5...a37611c. Read the comment docs.

@tensionhead
Copy link
Contributor

This got already reviewed as a sub branch, passing tests are enough for me here! Thx @dfsp-spirit for leading the way for streamlining the CRs :)

@tensionhead tensionhead merged commit 2a1504d into dev Jul 21, 2022
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