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

Check for user-supplied cache kwarg #113

Conversation

mattorourke17
Copy link
Contributor

@mattorourke17 mattorourke17 commented Feb 21, 2022

Fixes #112

simple fix to check for a user-supplied cache kwarg.

Sorry that this is showing so many commits. I let my fork get way out of date with the main develop branch, so all these commits were just me trying to get things back in consistency so I could make the (very simple) PR.

@mattorourke17 mattorourke17 changed the title Fixes #112 Check for user-supplied cache kwarg Feb 21, 2022
@codecov
Copy link

codecov bot commented Feb 21, 2022

Codecov Report

Merging #113 (36dedb7) into develop (28dc9dd) will decrease coverage by 2.11%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #113      +/-   ##
===========================================
- Coverage    79.19%   77.08%   -2.12%     
===========================================
  Files           39       39              
  Lines        15377    15379       +2     
===========================================
- Hits         12178    11855     -323     
- Misses        3199     3524     +325     
Impacted Files Coverage Δ
quimb/tensor/tensor_core.py 69.75% <50.00%> (-0.33%) ⬇️
quimb/linalg/slepc_linalg.py 10.41% <0.00%> (-79.77%) ⬇️
quimb/linalg/mpi_launcher.py 33.11% <0.00%> (-6.63%) ⬇️
quimb/tensor/optimize.py 61.74% <0.00%> (-4.59%) ⬇️
quimb/linalg/base_linalg.py 88.30% <0.00%> (-2.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28dc9dd...36dedb7. Read the comment docs.

@jcmgray
Copy link
Owner

jcmgray commented Feb 21, 2022

Unless I'm missing something, I'm not sure this patch will do anything, a function with the signature:

def foo(cache=True, **kwargs):
    ...

will never have cache in kwargs since its an explicit kwarg already?

I.e.

In [1]: def foo(cache=True, **kwargs):
   ...:     print(cache, kwargs)
   ...:

In [2]: foo(**{'cache': False})
False {}

@mattorourke17
Copy link
Contributor Author

ah yes, you are right. I made a simple mistake when testing this.

Sorry, you can close

@jcmgray jcmgray closed this Mar 10, 2022
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.

tensor_contract() interface prevents turning off contraction path cache
2 participants