Refactoring server.py and migrating to python-osc - #12
Conversation
ideoforms
left a comment
There was a problem hiding this comment.
Just spotted this PR - thanks so much!
I haven't yet tested or run the code, so this first review is mostly cosmetics. I'll follow up with a second review that takes a closer look at performance, tests etc.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
spyridonpallis
commented
Apr 22, 2023
Thanks for updating this guys. Any timeline on when this will be completed? |
Itaborala
commented
Apr 27, 2023
Hi @spyridonpallis , I've been very busy this month working on a time sensitive project. I should get back to this next week and hopefully we can have it completed in May. :) |
Itaborala
commented
Jul 6, 2023
@ideoforms Just letting you know I'm back to this, fully willing to finishing and having this publishes this month, if possible |
ideoforms
commented
Jul 14, 2023
@Itaborala That would be great! Let me know when you're at a point when this is ready for review with passing tests, and I'll take a final look over. |
ideoforms
commented
Dec 31, 2023
@Itaborala Let me know if you're still intending on finishing this up. No worries if not; I'd be happy to merge the changes and then tidy themself up myself. Thanks! |
Itaborala
commented
Jan 10, 2024
Hi @ideoforms , sorry, those busy days became some busy months.. Are there any other changes/additions to be made? |
ideoforms
left a comment
There was a problem hiding this comment.
Thanks for the PR! Just ran the tests and they're all passing fine. 🎉
I've added a few comments, main one being about restoring the previous approach to handling callback arguments - then good to merge.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
@ideoforms I've addressed all requests. Tests are working for me too. Additionally, I have noticed that in To account for |
ideoforms
left a comment
There was a problem hiding this comment.
Looks great @Itaborala! Merging now, thanks for all your work on this.
ideoforms
commented
Jan 12, 2024
@Itaborala I have just added tests for the On that note, I realised that part of the query system will now be broken following the move to python-osc. The callback system previously had callbacks indexed not only by the OSC address ( I don't think it should hold up a new release as it's probably a feature that's only needed in a few cases, but I'll add an issue for it. |
ideoforms
commented
Jan 12, 2024
I've now published this to PyPi as release v0.0.6: |
This should solve #9 and #11
Changes:
python-oscinstead ofpyliblofor OSCliblo,pylibloandCython.setup.pywas changed to 0.0.6, preparing for publishingFor approving this pull request, we need to make sure that the new server is fully backwards compatible with all other classes and methods and no bugs (originated by the refactoring) are present.