projectglow / glow

An open-source toolkit for large-scale genomic analysis
https://projectglow.io
Apache License 2.0
265 stars 111 forks source link

Remove typecheck for numpy arrays #366

Closed kianfar77 closed 3 years ago

kianfar77 commented 3 years ago

Signed-off-by: kianfar77 kiavash.kianfar@databricks.com

What changes are proposed in this pull request?

@typechecked directive from typeguard package does not support numpy arrays. It has an unpredictable behavior. For a safer behavior, this PR deactivates type checking for functions that have numpy array arguments. To keep type checking for the main entry points (RidgeReduction, LogisticRidgeRegression and RidgeRegression), I changed the alphas parameter type for them to List[float].

Also changed alpha_label_coef.label column to alpha_label_coef_label to avoid spark problem with dot in name

How is this patch tested?

(Details)

codecov[bot] commented 3 years ago

Codecov Report

Merging #366 (b718f61) into master (2bf0360) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #366   +/-   ##
=======================================
  Coverage   93.69%   93.69%           
=======================================
  Files          95       95           
  Lines        4823     4823           
  Branches      473      473           
=======================================
  Hits         4519     4519           
  Misses        304      304           
Impacted Files Coverage Δ
core/src/main/scala/io/projectglow/Glow.scala 95.65% <100.00%> (ø)

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 8b0bcd6...b718f61. Read the comment docs.

henrydavidge commented 3 years ago

Interesting, we use numpy type annotations extensively. We don't actually enforce the types anywhere else?

kianfar77 commented 3 years ago

Interesting, we use numpy type annotations extensively. We don't actually enforce the types anywhere else?

It shows an unpredictable behavior. For some functions it works. For some does not. It has caused problems for customers. I now deactivated it for more functions. For our entry point classes, I changed the aalphas type to keep the typecheck.

karenfeng commented 3 years ago

Thanks @kianfar77! Let's make sure to cut a new release for this.