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

[New Check]: TimeSeries rate is zero #404

Conversation

alessandratrapani
Copy link
Collaborator

@alessandratrapani alessandratrapani commented Sep 18, 2023

Related to Issue #389
Add a check function to check if the sampling rate is set to 0.0Hz

  • add check function
  • add unit tests
  • add check function documentation
  • update CHANGELOG.md

@alessandratrapani alessandratrapani changed the title Fix Issue #389 rate of TimeSeries is not zero Fix Issue #389 check if rate of TimeSeries is set to zero Sep 18, 2023
@alessandratrapani alessandratrapani added the category: new check a new best practices check to apply to all NWBFiles and their contents label Sep 18, 2023
@alessandratrapani alessandratrapani changed the title Fix Issue #389 check if rate of TimeSeries is set to zero [New Check]: TimeSeries rate is zero Sep 18, 2023
@alessandratrapani alessandratrapani self-assigned this Sep 18, 2023
@CodyCBakerPhD CodyCBakerPhD linked an issue Sep 18, 2023 that may be closed by this pull request
2 tasks
@CodyCBakerPhD
Copy link
Contributor

A couple things here:

(i) need unit tests

(ii) It occurs to me that for all of these PRs, you will have to run pre-commit manually since the bot does not work when the contributor is technically a GitHub organization (weird edge case with permissions that no one seems to be able to figure out - external forks are fine whenever they are an individual rather than an organization)

It's very simple though

pip install pre-commit
pre-commit install
pre-commit run -a

from a terminal while inside your clone of the CN fork of the repo

@CodyCBakerPhD CodyCBakerPhD marked this pull request as ready for review September 25, 2023 18:54
@CodyCBakerPhD
Copy link
Contributor

This looks good, just need a changelog entry (also if you approve #406 I can merge that in first so it can always be there to flag it on PRs)

@CodyCBakerPhD CodyCBakerPhD merged commit f43069b into NeurodataWithoutBorders:dev Sep 29, 2023
22 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the add-check-rate-of-timeseries-is-not-zero branch September 29, 2023 14:01
@bendichter
Copy link
Contributor

congratulations @alessandratrapani on your first contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: new check a new best practices check to apply to all NWBFiles and their contents
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Add Check]: rate of TimeSeries is not zero
3 participants