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

[Fix] Fixed failing complexity test #693

Merged

Conversation

rostro36
Copy link
Contributor

Description

Because misc/expspace.py was changed in this commit, the adjacent test now fails. As also proposed in #683 , I now also want to just adjust the numbers to what I get in a notebook. Also I linted the whole file.

If there are other (py)tests failing, I also want to clean that up over the weekend.

Proposed Changes

The interesting changes are on line 40 and 42 with new exact numbers. The rest is just linting; I also changed from a print message to an exception in line 634.

Checklist

Here are some things to check before creating the PR. If you encounter any issues, do let us know :)

  • I have read the CONTRIBUTING file.
  • My PR is targetted at the dev branch (and not towards the master branch).
  • I ran the CODE CHECKS on the files I added or modified and fixed the errors.
  • I have added the newly added features to News.rst (if applicable)

@DominiqueMakowski
Copy link
Member

sorry for the delay I just came back from vacations and it's pretty busy!
Thanks a lot for this PR. Overall, I think the test suite in neurokit could be improved, so don''t hesitate if you have any more ideas ☺️

@DominiqueMakowski DominiqueMakowski merged commit 9707b0f into neuropsychology:dev Aug 16, 2022
@welcome
Copy link

welcome bot commented Aug 16, 2022

landing
Congrats on merging your first pull request! 🎉🍾 We're looking forward to your next one!

@rostro36
Copy link
Contributor Author

No worries, I am grateful for all the work that you do for this project and did not expect to get answers right away.

I don't have that much experience with testing. There are a couple of other things I want to do first (like adding more RVT measures), but I can look into the test suite afterwards if I have time.

Generally, I think it already is good if there is an automated test suite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants