search

cherab / core

The core source repository for the Cherab project.
https://www.cherab.info
Other
45 stars 24 forks source link

Add Raysect 0.8 compatibility #379

Closed vsnever closed 1 year ago

vsnever commented 2 years ago

Raysect 0.8 has been released, so since the development branch should build against Raysect's master, the raysect-0.8-compat branch is ready to be merged into development.

jacklovell commented 2 years ago

I think it would be good to push out a Cherab release against 0.7.1 first, and then incorporate the changes required for 0.8 into a subsequent release. It's possible that the API breakage in 0.7.1 may break some users' downstream workflows if they use raysect features directly as well as Cherab.

vsnever commented 2 years ago

Yes, that's probably right. I'll leave this PR open for the future though.

jacklovell commented 1 year ago

Now that Cherab 1.4.0 is out I think we can look at updating the development branch for raysect 0.8. Tests all pass: any objections to the merge?

Mateasek commented 1 year ago

not from my side, thanks

vsnever commented 1 year ago

Just one thing, can we temporarily specify that we need matplotlib<=3.5.3, because this set_window_title() issue was fixed only in Raysect 0.8.1, which is not yet released?

jacklovell commented 1 year ago

Not too keen on working around raysect's bugs, but in this case setting a limit on the matplotlib version does seem like a pragmatic solution. I'll make the change.

CnlPepper commented 1 year ago

I'll release Raysect v0.8.1 this weekend, if I get time.

jacklovell commented 1 year ago

Thanks @CnlPepper. In that case I propose we wait until 0.8.1 is out and then just support that going forwards. I can't see anything in the 0.8.1 changelog that would affect Cherab: there are no instances of extract_translation or extract_rotation in Cherab's source code.

CnlPepper commented 1 year ago

@jacklovell I've released Raysect 0v.8.1. It fixes a number of niggling issues, such as the deprecated matplotlib window title call. If there are any further issues, let me know.

vsnever commented 1 year ago

@jacklovell, should we add the -O3 compilation flag like Raysect 0.8.1 does?

CnlPepper commented 1 year ago

I'd recommend you do. Anaconda and other python distributions seem to have changed their defaults to -o2, which results in a performance drop.

jacklovell commented 1 year ago

Interesting. Manylinux still uses -O3 which is what I build the PyPI wheels on, so forcing the use of -O3 in setup.py shouldn't make a difference for most users. I'll make this change and then merge.