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

Add complex.from_real_imag() #2064

Merged
merged 1 commit into from
Jan 9, 2025
Merged

Add complex.from_real_imag() #2064

merged 1 commit into from
Jan 9, 2025

Conversation

plajjan
Copy link
Contributor

@plajjan plajjan commented Jan 9, 2025

This staticmethod constructor allows us to create a complex instance
from two Reals.

Add a test suite for complex.

Fix division to raise exception on zero-divison

The acton.guide has been updated with a page on complex numbers.

Add new RAISE macro to simplify raising exceptions from C code.

@plajjan plajjan force-pushed the add-complex-from-real-imag branch from 7cf3bf8 to 645d787 Compare January 9, 2025 11:57
This staticmethod constructor allows us to create a complex instance
from two Reals.

Add a test suite for complex.

Fix division to raise exception on zero-divison

The acton.guide has been updated with a page on complex numbers.

Add new RAISE macro to simplify raising exceptions from C code.
@plajjan plajjan force-pushed the add-complex-from-real-imag branch from 645d787 to 3906613 Compare January 9, 2025 12:08
@plajjan plajjan changed the title Add complex.from_real_image() Add complex.from_real_imag() Jan 9, 2025
@plajjan
Copy link
Contributor Author

plajjan commented Jan 9, 2025

@sydow @nordlander FYI! I got complex working by adding this from_real_imag() constructor. Added a test suite and found the division-by-zero not raising exception, so I fixed that and also realized our $RAISE is very ugly so added a nicer macro around that. One test is disabled as it depends on fromatom, which is not implemented and I'm not sure how to proceed there. With a little guidance, maybe I could give that a shot.

Also added a bit of documentation. I have used the Claude AI both for producing the test suite and docs and while I strive to proof read and guide the LLM, I am not proficient enough in the subject to feel entirely confident about the result. I will merge this now, because @mzagozen needs it for his advent-of-code and given the test suite I am quite confident this works well enough, but please feel free to read so we can fix any potential issues :)

@plajjan plajjan merged commit 719b0be into main Jan 9, 2025
25 checks passed
@plajjan plajjan deleted the add-complex-from-real-imag branch January 9, 2025 12:34
@sydow
Copy link
Collaborator

sydow commented Jan 9, 2025 via email

@plajjan
Copy link
Contributor Author

plajjan commented Jan 11, 2025

@sydow ok, I've re-enabled the test according to your suggestion in #2070! Thanks!

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