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

Add PointProcess class #43

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

hokiedsp
Copy link
Contributor

@hokiedsp hokiedsp commented Feb 17, 2021

Here is the renamed PR for PointProcess class.

I renamed my mydev branch to add-pointprocess-class and it apparently closed the original PR automatically.

CrossRef: #41,

TODOs

  • Complete pytest
  • Complete docstrings
  • Add to_PointProcess() methods to the source classes
  • Add PointProcess.to_XXX() methods to convert from PointProcess
  • Add PointProcess xtor taking a sequence-like object (which implements __len__() and __getitem__(int)
  • Make PointProcess a sequence-like class by implementing __len__() and __getitem__(int).

@YannickJadoul
Copy link
Owner

Ah, oof :-)
Let's quickly cross-reference #41, then

Copy link
Owner

@YannickJadoul YannickJadoul left a comment

Choose a reason for hiding this comment

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

This is great; I hadn't realized it was such a nicely contained set of changes!

What I've done so far in the two extra commits (go ahead and check) is mainly making sure that the full diff doesn't show difference that are just indentation or style changes. Now that there's a contributor, I should definitely get a .clang-format file, but adding and applying that is probably best left outside of this PR (to ensure we focus on the important parts and content here!).

Also, a few things to discuss that I've noticed. Apart from that, I'll dive into the details soon, but something tells me I won't have a lot of work to do :-)

src/parselmouth/Pitch.cpp Outdated Show resolved Hide resolved
src/parselmouth/Pitch_docstrings.h Show resolved Hide resolved
src/parselmouth/PointProcess_docstrings.h Outdated Show resolved Hide resolved
tests/test_pointprocess.py Outdated Show resolved Hide resolved
@hokiedsp hokiedsp force-pushed the add-pointprocess-class branch from e37d662 to ed911bf Compare February 27, 2021 04:18
@hokiedsp
Copy link
Contributor Author

hokiedsp commented Feb 28, 2021

The last commit should truly be the last commit to this PR from me. Added Sound_docstings.h to round out documentations involving PointProcess class

@YannickJadoul YannickJadoul force-pushed the add-pointprocess-class branch 2 times, most recently from af4b1f6 to 19bde38 Compare February 28, 2021 21:13
@YannickJadoul YannickJadoul force-pushed the master branch 2 times, most recently from 36b9043 to b6595d3 Compare June 14, 2024 12:01
@hokiedsp hokiedsp force-pushed the add-pointprocess-class branch 2 times, most recently from f121d88 to 359b8ba Compare October 4, 2024 00:18
hokiedsp and others added 16 commits October 3, 2024 19:20
- shimmer functions
- alternate constructors
- create_poisson_process()
- get_time_points()
- add_points()
Moved lines around for readability
Removed _t & _nt
Added method docstrings
get_count_and_fraction_of_voice_breaks() returns 4-element tuple
Added PointProcess.__len__(), .__getitem__(), & __iter__()
Updated test
Fixed PointProcess empty constructor
pytest: dropped 'eee.wav', using global fixtures
to_text_grid_vuv(), to_sound_phonation(), to_sound_pulse_train(), to_sound_hum(),
- Added class docstring and docstrings of new methods
@hokiedsp hokiedsp force-pushed the add-pointprocess-class branch from 359b8ba to dff9aa9 Compare October 4, 2024 00:30
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