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.

Sign up for GitHub

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

Jump to bottom

291 show select #301

Merged
merged 5 commits into from
Jul 8, 2022
Merged

291 show select #301

merged 5 commits into from
Jul 8, 2022

Conversation

tensionhead
Copy link
Contributor

Author Guidelines

  • Is the change set < 400 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?

Adresses #291

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?

@tensionhead
FIX: Catch invalid show kwargs
9bf9353 
- for show we only support simple indexing (unique and ordered)

On branch 291-show-select
Changes to be committed:
	modified:   syncopy/datatype/methods/show.py
@tensionhead
NEW: Test for invalid show kwargs
f906cd5 
- catch non-unique and/or not sorted indices

On branch 291-show-select
Changes to be committed:
	modified:   syncopy/tests/test_selectdata.py
@tensionhead
FIX: Catch also out-of-range toi selections in show
21849a8 
- toi is a special case, as out of range intervals [X, Y] get mapped
to the last valid index [Z, Z] which then is invalid

On branch 291-show-select
Changes to be committed:
	modified:   syncopy/datatype/methods/show.py
	modified:   syncopy/tests/test_selectdata.py
@tensionhead
FIX: Check for sorted/unique input for selections with string sequences
4ebbc61 
- channel selections can be ['channel7', 'channel3'], we have to catch
this as well
- in the future it might be good to allow for unordered plotting

On branch 291-show-select
Changes to be committed:
	modified:   syncopy/datatype/methods/show.py
	modified:   syncopy/tests/test_plotting.py
@tensionhead tensionhead requested a review from dfsp-spirit July 8, 2022 09:42
@tensionhead
FIX: refined toi index check
b0aad5a 
- toi for the time/stacking axis is special

Changes to be committed:
	modified:   syncopy/datatype/methods/show.py
@codecov
Copy link

codecov bot commented Jul 8, 2022

Codecov Report

Merging #301 (b0aad5a) into dev (2e3080f) will decrease coverage by 0.15%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##              dev     #301      +/-   ##
==========================================
- Coverage   71.16%   71.00%   -0.16%     
==========================================
  Files          68       68              
  Lines        7882     7913      +31     
  Branches     1620     1631      +11     
==========================================
+ Hits         5609     5619      +10     
- Misses       1885     1900      +15     
- Partials      388      394       +6
Impacted Files Coverage Δ
syncopy/datatype/methods/show.py 81.35% <81.81%> (-0.79%) ⬇️
syncopy/specest/freqanalysis.py 61.61% <0.00%> (-4.05%) ⬇️
syncopy/specest/compRoutines.py 77.44% <0.00%> (-2.13%) ⬇️
syncopy/datatype/base_data.py 83.17% <0.00%> (+0.17%) ⬆️

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 2e3080f...b0aad5a. Read the comment docs.

@dfsp-spirit dfsp-spirit linked an issue Jul 8, 2022 that may be closed by this pull request
show() and selectdata() keywords #291
Closed
dfsp-spirit
dfsp-spirit approved these changes Jul 8, 2022
View reviewed changes
Copy link
Collaborator

@dfsp-spirit dfsp-spirit left a comment

Choose a reason for hiding this comment

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

looking good

@dfsp-spirit dfsp-spirit merged commit 95b6eb2 into dev Jul 8, 2022
@tensionhead tensionhead deleted the 291-show-select branch July 12, 2022 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dfsp-spirit dfsp-spirit dfsp-spirit approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

show() and selectdata() keywords
2 participants
@tensionhead @dfsp-spirit