-
Notifications
You must be signed in to change notification settings - Fork 127
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/table rows #884
Fix/table rows #884
Conversation
Codecov Report
@@ Coverage Diff @@
## main #884 +/- ##
==========================================
+ Coverage 59.23% 59.30% +0.07%
==========================================
Files 172 172
Lines 10641 10641
==========================================
+ Hits 6303 6311 +8
+ Misses 4338 4330 -8 |
I have realized that this fails: >>> dimx = 10
>>> dimy = 1
>>> my_conv = np.random.rand(dimx, dimy)
>>> mapdl.load_array("my_conv", my_conv)
>>> assert np.allclose(mapdl.parameters["my_conv"], my_conv) #Fail
AssertionError Traceback (most recent call last)
<ipython-input-13-0db9fb3fa50d> in <module>
>>> mapdl.parameters["my_conv"].shape
(10,)
>>> my_conv.shape
(1, 10) But this works >>> dimx = 1
>>> dimy = 10
>>> my_conv = np.random.rand(dimx, dimy)
>>> mapdl.load_array("my_conv", my_conv)
>>> assert np.allclose(mapdl.parameters["my_conv"], my_conv) #Works
True
>>> mapdl.parameters["my_conv"].shape
(10,)
>>> my_conv.shape
(10, 1) I could remove the squeeze of this line: However, then checking against a list will fail because: >>> my_conv = [1,2,3]
>>> mapdl.load_array("my_conv", my_conv)
>>> assert np.allclose(mapdl.parameters["my_conv"], my_conv) #Works
False
>>> mapdl.parameters["my_conv"].shape
(3,1)
>>> my_conv.shape
(3,) I guess we need to choose between the right assertions: If the former, list assertions will fail because they are initialized as I feel we should go for the later, return always the second dimension even if it is 1. |
I remember writing this thinking that we might have this issue. Agree that we should be consistent, pick one and go with it (and I'm leaning
Implement and remind me to review. |
@akaszynski ... feel free to review again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Close #883.