search

MaxOhn / rosu-pp-py

osu! difficulty and pp calculation for all modes
MIT License
23 stars 19 forks source link

similar #1

Closed Pure-Peace closed 2 years ago

Pure-Peace commented 2 years ago

similar https://github.com/Pure-Peace/peace-performance-python lol

Pure-Peace commented 2 years ago

May help you e.g automatically build and publish to pip through workflow

MaxOhn commented 2 years ago

Yeah I was aware of your repo, impressive details in the readme. I just saw that yours wasn't updated to the latest performance changes and I wasn't sure if there were any other internal changes you made.

I figured it would be nice to have a very simple repo with a simple interface as default that I could update immediatly after making changes to the main calculations. I do love seeing other people make their own bindings though or even building onto mine since I'm not really planning to bring any more complexity into this.

I've never setup any CI since I didn't see the need but I think these bindings might actually be a solid use-case so I might look into it sometime and let you know.

Pure-Peace commented 2 years ago

Yeah I was aware of your repo, impressive details in the readme. I just saw that yours wasn't updated to the latest performance changes and I wasn't sure if there were any other internal changes you made.

I figured it would be nice to have a very simple repo with a simple interface as default that I could update immediatly after making changes to the main calculations. I do love seeing other people make their own bindings though or even building onto mine since I'm not really planning to bring any more complexity into this.

I've never setup any CI since I didn't see the need but I think these bindings might actually be a solid use-case so I might look into it sometime and let you know.

I initially wanted to make python bindings for rosu-pp, but there were a few things that needed to be modified, so I created branches. (actually my branch changes are not that big.) e.g: 1. synchronous beatmap parsing is supported by default, and it's nice to add additional async methods when you turn on async features, instead of having to choose between synchronous and asynchronous.

  1. there are a few variables that need to be exposed (raw pp: eg. aim acc, spd).
  2. add features to support other developers to modify or create their own algorithm to modify.(This can actually be achieved with a branch)

CI is very useful because it can automatically build pip packages available on all platforms at once https://github.com/Pure-Peace/peace-performance-python/releases/tag/release-v1.1.2 Anyone who wants to use only needs pip install

I am more than willing to make rosu-pp python bindings if you can accept some minor changes to rosu-pp

MaxOhn commented 2 years ago
  1. synchronous beatmap parsing is supported by default, and it's nice to add additional async methods when you turn on async features, instead of having to choose between synchronous and asynchronous.

Async makes a lot of sense in a bunch of fields like networking or IO interactions. Since map parsing is basically just IO, making that async is pretty good. Everything else in difficulty / performance calculations doesn't really benefit from async though so no other function is async. If people still want to execute those functions concurrently, they can wrap them in an async { ... } so I don't feel like this is sufficient cause to provide the methods as async even when either of the two async features are enabled. Regarding python's async, I'm just not familiar enough with the language to add rust-like feature flags to python and admittedly I'm also not invested enough to setup benchmarking to compare performances but yeah it might be worth a look and make map parsing in python async too.

  1. there are a few variables that need to be exposed (raw pp: eg. aim acc, spd).

I forgot how exactly it was before but I did some adjustments to the attributes structure for difficulty and performance results. Now fields are definitely all marked pub so I'm assuming that's no longer an issue?

  1. add features to support other developers to modify or create their own algorithm to modify.(This can actually be achieved with a branch)

Not sure what kind of features you mean. Custom algorithms are best kept in new forks anyway.

CI is very useful because it can automatically build pip packages available on all platforms at once https://github.com/Pure-Peace/peace-performance-python/releases/tag/release-v1.1.2 Anyone who wants to use only needs pip install

I'm not very familiar with the whole python eco- or packaging system. My current binding setup seemed to work including pip install but I didn't look too closely into wheels and all that other python dependency business. Probably worth a look though.

I am more than willing to make rosu-pp python bindings if you can accept some minor changes to rosu-pp

Feel free to PR whatever change you think might be useful for any party, be it the enduser or developper. I'm happy to consider them and likely add them one way or another 😄

All in all, I'm not really planning to extend the bindings much though. The plan was to provide a simple interface to the actual rust calculations and keep it at that. Depending what your own goal is with a python binding, you might be better off maintaining your own version.

Pure-Peace commented 2 years ago
  1. synchronous beatmap parsing is supported by default, and it's nice to add additional async methods when you turn on async features, instead of having to choose between synchronous and asynchronous.

Async makes a lot of sense in a bunch of fields like networking or IO interactions. Since map parsing is basically just IO, making that async is pretty good. Everything else in difficulty / performance calculations doesn't really benefit from async though so no other function is async. If people still want to execute those functions concurrently, they can wrap them in an async { ... } so I don't feel like this is sufficient cause to provide the methods as async even when either of the two async features are enabled. Regarding python's async, I'm just not familiar enough with the language to add rust-like feature flags to python and admittedly I'm also not invested enough to setup benchmarking to compare performances but yeah it might be worth a look and make map parsing in python async too.

  1. there are a few variables that need to be exposed (raw pp: eg. aim acc, spd).

I forgot how exactly it was before but I did some adjustments to the attributes structure for difficulty and performance results. Now fields are definitely all marked pub so I'm assuming that's no longer an issue?

  1. add features to support other developers to modify or create their own algorithm to modify.(This can actually be achieved with a branch)

Not sure what kind of features you mean. Custom algorithms are best kept in new forks anyway.

CI is very useful because it can automatically build pip packages available on all platforms at once https://github.com/Pure-Peace/peace-performance-python/releases/tag/release-v1.1.2 Anyone who wants to use only needs pip install

I'm not very familiar with the whole python eco- or packaging system. My current binding setup seemed to work including pip install but I didn't look too closely into wheels and all that other python dependency business. Probably worth a look though.

I am more than willing to make rosu-pp python bindings if you can accept some minor changes to rosu-pp

Feel free to PR whatever change you think might be useful for any party, be it the enduser or developper. I'm happy to consider them and likely add them one way or another 😄

All in all, I'm not really planning to extend the bindings much though. The plan was to provide a simple interface to the actual rust calculations and keep it at that. Depending what your own goal is with a python binding, you might be better off maintaining your own version.

Thanks for your reply!