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

Introduce missing Clip properties to run-console.py + view.py #143

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

Conversation

esaruoho
Copy link
Contributor

Hi @ideoforms
Since I can't wrap my mind around the "view clip or view device on track" mode change, but you did point me to the unofficial PythonLiveAPI - I'm trying to do the lowest hanging fruit, which is this:

this PR introduces the following properties to clip.py + autocomplete in run-console.py:

has_groove
is_overdubbing
sample_length
start_time
will_record_on_start
color_index
launch_mode
launch_quantization
legato
muted
position
ram_mode
velocity_amount
warp_mode

i also added some comments to the code that might help with where it could be taken later - i.e. since some of the commented away properties actually return something that is not processed properly by run-console.py - i.e. error "Infered arg_value type is not supported" - maybe someone can take that over the finishing line.

do let me know if this type of stuff is welcome. i think the position one is amazing! warp_mode was weird tho, with number 5 shooting errors whereas 6 shoots "Complex Pro" without issues.

if this is welcome, I'll run through the rest of the functions and try and make small increment tweaks until they amount to something.

has_groove
is_overdubbing
sample_length
start_time
will_record_on_start
color_index
launch_mode
launch_quantization
legato
muted
position
ram_mode
velocity_amount
warp_mode
@esaruoho
Copy link
Contributor Author

added clip properties to documentation too.

@esaruoho
Copy link
Contributor Author

esaruoho commented Oct 2, 2024

@ideoforms what do you think, is this good to go?

@esaruoho
Copy link
Contributor Author

esaruoho commented Nov 9, 2024

@ideoforms ping - does this PR make any sense for you? it seems to me that this was a good start at adding more stuff in there, and i left the TODOs documented for someone else to pick up.

I'd love to continue working on adding more missing properties/controls for other parts of AbletonOSC - but just need your green light really.

@esaruoho
Copy link
Contributor Author

Hi @ideoforms , any take on this? I was hoping to introduce more PRs with more additions but have been waiting to see what happens with this first before putting more work in :)

Copy link
Owner

@ideoforms ideoforms left a comment

Choose a reason for hiding this comment

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

Hi @esaruoho , thanks for your work on this! The properties I've tested work fine.
Main issues:

  • Most of the comments can be removed for consistency with the rest of the codebase. As you've already documented the return types in the README (which is ace, and should really happen for all other the properties at some point!), no need to document them here here too
  • All of the new properties need unit tests adding, with new tests in test_clip.py. There is some information on running the unit test suite in CONTRIBUTING.md. Let me know if you run into any issues.

I've added a few more comments inline. Thanks!

abletonosc/clip.py Outdated Show resolved Hide resolved
abletonosc/clip.py Outdated Show resolved Hide resolved
abletonosc/clip.py Outdated Show resolved Hide resolved
abletonosc/clip.py Outdated Show resolved Hide resolved
abletonosc/clip.py Outdated Show resolved Hide resolved
abletonosc/clip.py Outdated Show resolved Hide resolved
abletonosc/clip.py Outdated Show resolved Hide resolved
abletonosc/clip.py Outdated Show resolved Hide resolved
abletonosc/clip.py Outdated Show resolved Hide resolved
abletonosc/clip.py Outdated Show resolved Hide resolved
@esaruoho
Copy link
Contributor Author

@ideoforms thanks! i've addressed the comments with a new commit.

if you're okay with the new commit, please let me know, and i can try and have a look at test_clip.py, but thinking about it makes my hands shake. i hope i'll find a slice of time to do that, too.

@ideoforms
Copy link
Owner

Changes look great, thanks!
You'll do great on the tests. Best way is to start with a really simple one, just by copying one of the existing tests (e.g. try writing a test for color_index based on the one for color), and then build from there.

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