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

Fix pretty printing of polynomials (mostly) #20

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jakewilliami
Copy link

  1. Made unicode super- and subscripts more compact

  2. Performance considerations using print(io, ...) instead of join.

  3. Code cleanup. Merged the print_poly function directly into Base.show, as print_poly was only being used in the show definition (i.e., the function was redundant).

  4. The pretty_var function was not printing correctly, causing tests to fail—the reason for this is because polynomials would sometimes print in the form

2z₂⁴

But sometimes of the form

2z[2]⁴

I'm not sure what caused that, nor for how long the tests were broken, but I just added an extra regex matching in the pretty_var function to capture this. The only thing failing now is converting variables of Polynomial to Symbols correctly:

poly: Test Failed at /home/jakewilliami/projects/FixedPolynomials.jl/test/poly_test.jl:41
  Expression: variables(F[1]) == [:z1, :z2]
   Evaluated: [Symbol("z[1]"), Symbol("z[2]")] == [:z1, :z2] 
# ...
Test Summary: | Pass  Fail  Total
poly          |   50     1     51
ERROR: LoadError: LoadError: Some tests did not pass: 50 passed, 1 failed, 0 errored, 0 broken.

Sorry I haven't been able to fix the Symbol problem. It would be good to know what changes caused Polynomials to print in 2 different ways.

  1. Merged the `print_poly` function directly into `Base.show`, as `print_poly` was only being used in the `show` definition (i.e., the function was redundant).

  2. The `pretty_var` function was not printing correctly, causing tests to fail—the reason for this is because polynomials would sometimes print in the form
```julia
2z₂⁴
```
But sometimes of the form
```julia
2z[2]⁴
```
I'm not sure what caused that, nor for how long the tests were broken, but I just added an extra regex matching in the `pretty_var` function to capture this.  The only thing failing now is converting variables of `Polynomial` to `Symbol`s correctly:
```julia
poly: Test Failed at /home/jakewilliami/projects/FixedPolynomials.jl/test/poly_test.jl:41
  Expression: variables(F[1]) == [:z1, :z2]
   Evaluated: [Symbol("z[1]"), Symbol("z[2]")] == [:z1, :z2] 
# ...
Test Summary: | Pass  Fail  Total
poly          |   50     1     51
ERROR: LoadError: LoadError: Some tests did not pass: 50 passed, 1 failed, 0 errored, 0 broken.
```
@jakewilliami
Copy link
Author

By the way, it would be nice to have a Project.toml file I think 🙂 This will replace REQUIRE.

@jakewilliami
Copy link
Author

Another small performance consideration that I can't do right now, but you could consider using view where possible (i.e., in the calculation of degree:

julia> @btime sum(exponents(p)[:, 1]) # current implementation of `degree`
  41.922 ns (1 allocation: 112 bytes)
4

julia> @btime @inbounds sum(view(exponents(p), :, 1)) # `degree` using view
  15.358 ns (0 allocations: 0 bytes)
4

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.

1 participant