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

Simplify NI-FGEN example and remove its use of numpy and scipy #1817

Merged
merged 17 commits into from
Aug 30, 2022

Conversation

ni-jfitzger
Copy link
Collaborator

@ni-jfitzger ni-jfitzger commented Aug 27, 2022

  • This contribution adheres to CONTRIBUTING.md.

  • I've updated CHANGELOG.md if applicable.

  • [ ] I've added tests applicable for this pull request

What does this Pull Request accomplish?

  • Eliminates our use of numpy and scipy from the NI-FGEN example, and slightly simplifies the waveform creation, to keep the focus on the usage of NI-FGEN.
    • The new waveforms will only be a single cycle. Previous waveforms were an arbitrary 1.x cycles, because the phase ended at 10, which is greater than 2PI and not even a multiple of 2PI.
  • The Triangle waveform in the example was actually a sawtooth. We will now refer to it as such.
    • I could have added an actual triangle wave, but doing so with 0 offset would have been complicated enough that I worried I felt it would distract from the focus on the API usage. 3 or 4 lines, probably, but still the most complicated of the waveforms.
  • Scipy has stopped releasing Windows 32-wheels for scipy 1.8.0+ and for Python 3.10 and later. This is breaking nimi-python testing for PRs, when we try to add Python 3.10 support. This change fixes that.
    • Side note: building the scipy Windows 32 wheel would be non-trivial.

List issues fixed by this Pull Request below, if any.

What testing has been done?

  • Hand-tested updated functions, because they are part of the example code and not unit-tested.
    • Examined the output (in list form) from the updated functions and they all look correct to me.
    • I plotted the noise waveform and it looked good.
    • I generated a 1000 point noise waveform with the same algorithm and I think the max absolute value was around 0.8. It never exceed the bounds of [-1, 1].

CHANGELOG.md Outdated Show resolved Hide resolved
@ni-jfitzger ni-jfitzger changed the title Remove scipy dependency from nimi-python Simplify NI-FGEN example and remove its use of numpy and scipy Aug 29, 2022
src/nifgen/examples/nifgen_script.py Outdated Show resolved Hide resolved
src/nifgen/examples/nifgen_script.py Outdated Show resolved Hide resolved
src/nifgen/examples/nifgen_script.py Outdated Show resolved Hide resolved
ni-jfitzger and others added 3 commits August 30, 2022 08:50
Make sawtooth wave correct length

Co-authored-by: Marcos <marcoskirsch@users.noreply.github.com>
@marcoskirsch marcoskirsch merged commit c4106c5 into ni:master Aug 30, 2022
@ni-jfitzger ni-jfitzger deleted the remove_scipy_dependency branch August 30, 2022 15:06
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.

nimi-python uses scipy which keeps us from adding support to Python 3.10
2 participants