[GitHub] madlib issue #292: Internal: Add function to check column type for 1D array

[ArvindSridhar]

Github user ArvindSridhar commented on the issue:

https://github.com/apache/madlib/pull/292
  
PR ready to be closed, changes made elsewhere already


---

[GitHub] madlib issue #292: Internal: Add function to check column type for 1D array

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/madlib/pull/292
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/madlib-pr-build/559/



---

[GitHub] madlib issue #292: Internal: Add function to check column type for 1D array

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/madlib/pull/292
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/madlib-pr-build/558/



---

[GitHub] madlib issue #292: Internal: Add function to check column type for 1D array

[ArvindSridhar]

Github user ArvindSridhar commented on the issue:

https://github.com/apache/madlib/pull/292
  
Sounds good. Just pushed a commit to this PR, will rebase and add back the 
original function to the original vec2cols PR tomorrow


---

[GitHub] madlib issue #292: Internal: Add function to check column type for 1D array

[iyerr3]

Github user iyerr3 commented on the issue:

https://github.com/apache/madlib/pull/292
  
Yeah, the unit tests are unnecessary after this change. I vote to remove
them, with just the function added back to the original PR.



On Thu, Jul 12, 2018 at 5:26 PM Arvind Sridhar 
wrote:

> Sounds good-just verified that plpy converts the SQL bool into a Python
> bool and not a Python string. It should be good to go now. Do we still 
want
> to get rid of the unit tests now given that they would be almost trivial
> (basically just checking that True=True)?
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> , or 
mute
> the thread
> 

> .
>



---

[GitHub] madlib issue #292: Internal: Add function to check column type for 1D array

[ArvindSridhar]

Github user ArvindSridhar commented on the issue:

https://github.com/apache/madlib/pull/292
  
Sounds good-just verified that plpy converts the SQL bool into a Python 
bool and not a Python string. It should be good to go now. Do we still want to 
get rid of the unit tests now given that they would be almost trivial 
(basically just checking that True=True)?


---

[GitHub] madlib issue #292: Internal: Add function to check column type for 1D array

[iyerr3]

Github user iyerr3 commented on the issue:

https://github.com/apache/madlib/pull/292
  
That's a valid concern. You could return bool(result[0]['n_y']) to ensure a
bool output instead of a string (though I believe the SPI ensures that
boolean from SQL is returned as bool in Python).

>



---

[GitHub] madlib issue #292: Internal: Add function to check column type for 1D array

[iyerr3]

Github user iyerr3 commented on the issue:

https://github.com/apache/madlib/pull/292
  
Few comments: 
1. The query would read better if it is setup to return a boolean i.e. 
using `array_upper(..., 2) IS NULL` instead of converting the integer output to 
bool in Python. 
2. My request in #291 for moving this to a separate commit was for the 
`plpy_mock.py_in` file since that looks like an independent work. This function 
is minor enough that it can be kept in #291. 
3. The unit test for this function isn't really testing anything except the 
last line in the function. That also becomes moot if comment 1. is accepted. 
Considering this, I suggest we don't have the unit test for this function (at 
least not in the form it currently is). 


---

[GitHub] madlib issue #292: Internal: Add function to check column type for 1D array

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/madlib/pull/292
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/madlib-pr-build/554/



---

[GitHub] madlib issue #292: Internal: Add function to check column type for 1D array

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/madlib/pull/292
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/madlib-pr-build/553/



---