Skip to content

Add Raysect 0.8 compatibility#379

Merged
jacklovell merged 7 commits into
developmentfrom
raysect-0.8-compat
Feb 14, 2023
Merged

Add Raysect 0.8 compatibility#379
jacklovell merged 7 commits into
developmentfrom
raysect-0.8-compat

Conversation

@vsnever
Copy link
Copy Markdown
Member

@vsnever vsnever commented Sep 13, 2022

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.

Only changes needed to build the project without errors are included.
@vsnever vsnever marked this pull request as draft September 13, 2022 16:52
@jacklovell
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Member Author

vsnever commented Sep 14, 2022

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

@jacklovell jacklovell added this to the 1.5 milestone Dec 22, 2022
@jacklovell jacklovell marked this pull request as ready for review February 8, 2023 16:55
@jacklovell
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Member

Mateasek commented Feb 9, 2023

not from my side, thanks

@vsnever
Copy link
Copy Markdown
Member Author

vsnever commented Feb 9, 2023

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
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Member

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

@jacklovell
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Member

CnlPepper commented Feb 12, 2023

@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
Copy link
Copy Markdown
Member Author

vsnever commented Feb 13, 2023

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

@CnlPepper
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Member

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.

Recent Anaconda and other Python distributions have changed to -O2
which results in a performance degradation. Manylinux and older
Anacondas use -O3, so specify this to maintain behaviour.
@jacklovell jacklovell merged commit 793c4c6 into development Feb 14, 2023
@jacklovell jacklovell deleted the raysect-0.8-compat branch February 15, 2023 10:37
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.

4 participants