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

Tidy src/atoms/affine #538

Merged
merged 1 commit into from
Jan 10, 2024
Merged

Tidy src/atoms/affine #538

merged 1 commit into from
Jan 10, 2024

Conversation

odow
Copy link
Member

@odow odow commented Jan 10, 2024

Just some small formatting changes, mainly so I have an excuse to read through and understand the code.

This was referenced Jan 10, 2024
Copy link

codecov bot commented Jan 10, 2024

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (8dfc7f6) 92.83% compared to head (dda073e) 92.73%.

Files Patch % Lines
src/atoms/affine/partialtranspose.jl 46.66% 8 Missing ⚠️
src/atoms/affine/diagm.jl 70.00% 3 Missing ⚠️
src/atoms/affine/diag.jl 84.61% 2 Missing ⚠️
src/atoms/affine/inner_product.jl 0.00% 2 Missing ⚠️
src/atoms/affine/conjugate.jl 83.33% 1 Missing ⚠️
src/atoms/affine/index.jl 90.00% 1 Missing ⚠️
src/atoms/affine/multiply_divide.jl 96.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #538      +/-   ##
==========================================
- Coverage   92.83%   92.73%   -0.11%     
==========================================
  Files          86       86              
  Lines        5375     5310      -65     
==========================================
- Hits         4990     4924      -66     
- Misses        385      386       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

end

head(io::IO, ::ImaginaryAtom) = print(io, "imag")

function Base.sign(x::ImaginaryAtom)
# FIXME(odow): what is this?
sign(x.children[1]) == ComplexSign()
Copy link
Member Author

@odow odow Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow up later: I don't think this method is correct because this statement isn't used.

@odow odow mentioned this pull request Jan 10, 2024
# add_subtract.jl
# Handles unary negation, addition and subtraction of variables, constants
# and expressions.
# All expressions and atoms are subtpyes of AbstractExpr.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not useful or has it be moved somewhere ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think it was useful. It's pretty obvious that NegateAtom handles negation. And we really need a docstring for each atom with examples, rather than this vague comment in the file. But that's a separate PR...

Base automatically changed from od/fix-imports to master January 10, 2024 20:21
@odow odow merged commit d856e42 into master Jan 10, 2024
8 of 10 checks passed
@odow odow deleted the od/atoms-affine branch January 10, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants