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

test expected output #486

Merged
merged 4 commits into from
Feb 28, 2024
Merged

Conversation

Bchass
Copy link
Contributor

@Bchass Bchass commented Feb 23, 2024

Description

Fixes #471

Changes

  • update examples to try and line up with what doctest expects

Checklist

  • Use ruff and pylint for errors related to code style and formatting.
  • Verify all previous and newly added unit tests pass in pytest.
  • Check the documentation build does not lead to any failures. Sphinx build can be checked locally for any failures related to your PR
  • Use linkcheck to check for broken links in the documentation
  • Use doctest to verify the examples in the function docstrings work as expected.

@purva-thakre
Copy link
Collaborator

purva-thakre commented Feb 23, 2024

You might want to rebase your PR. What you're working with is from an older commit.

Also, you might have to make changes to the code of the function to get to array output (probably) which is what I referred to in the issue discussion.

image

@purva-thakre
Copy link
Collaborator

purva-thakre commented Feb 24, 2024

@Bchass After you rebase this branch, you could try to figure out where the function changes a numpy array to a 2D list at each step by making python print the variable type after each step.

p_mat = np.zeros((n, n), dtype=int)
np.fill_diagonal(p_mat[1:], 1)
p_mat[0, -1] = 1
result_mat = np.linalg.matrix_power(p_mat, k)
return result_mat

For example, temporarily use print(type(variable_name)) after each line in the function code. If you use the function, you should be able see where the function transitions from manipulating a np.ndarray type to a 2D list.

Hopefully, this makes sense? Again, I haven't had the chance to dig into the issue yet but this is what my first step would have been.

@Bchass
Copy link
Contributor Author

Bchass commented Feb 24, 2024

Thanks for the info! I'll look into it more at some point this weekend.

@Bchass
Copy link
Contributor Author

Bchass commented Feb 25, 2024

@purva-thakre Stepping through the function, it never converted to a 2D list. It boiled down to whitespace and how I was calling the function in the examples.

Came across: https://numpy.org/doc/stable/reference/generated/numpy.set_printoptions.html and was able to format the result how doctest expects it.

Edit: resolved the conflicts but the docstring job is now failing. Looks like the changes I made are breaking some other examples. I'll look into this furher.

Copy link

codecov bot commented Feb 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.1%. Comparing base (f613473) to head (812288d).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #486     +/-   ##
========================================
- Coverage    98.1%   98.1%   -0.1%     
========================================
  Files         161     161             
  Lines        3096    3095      -1     
  Branches      753     752      -1     
========================================
- Hits         3038    3037      -1     
  Misses         37      37             
  Partials       21      21             

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

@purva-thakre
Copy link
Collaborator

Stepping through the function, it never converted to a 2D list. It boiled down to whitespace and how I was calling the function in the examples.

That's interesting!

@Bchass
Copy link
Contributor Author

Bchass commented Feb 27, 2024

More failures to investigate tomorrow

@purva-thakre
Copy link
Collaborator

purva-thakre commented Feb 27, 2024

@Bchass You can ignore the drop in pytest coverage. The linkcheck workflow passes now, previous failure could have been related to the website being down or something.

@Bchass Bchass marked this pull request as ready for review February 28, 2024 01:39
@purva-thakre purva-thakre self-requested a review February 28, 2024 03:53
@purva-thakre purva-thakre merged commit fcbf5d2 into vprusso:master Feb 28, 2024
33 of 34 checks passed
@purva-thakre purva-thakre added this to the v1.0.8 milestone Mar 28, 2024
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.

Output of toqito/matrices/cyclic_permutation.py is a 2D list
2 participants