JuliaML / TableTransforms.jl

Transforms and pipelines with tabular data in Julia
https://juliaml.github.io/TableTransforms.jl/stable
MIT License
103 stars 15 forks source link

Replace the `assert` macro with the new `_assert` utility function #226

Closed eliascarv closed 9 months ago

eliascarv commented 9 months ago

Motivation: @assert docstring:

help?> @assert
  @assert cond [text]

  Throw an AssertionError if cond is false. Preferred syntax for writing
  assertions. Message text is optionally displayed upon assertion failure.

  │ Warning
  │
  │  An assert might be disabled at various optimization levels. Assert
  │  should therefore only be used as a debugging tool and not used for
  │  authentication verification (e.g., verifying passwords), nor should
  │  side effects needed for the function to work correctly be used
  │  inside of asserts.
codecov-commenter commented 9 months ago

Codecov Report

Merging #226 (97b0228) into master (beb0c2a) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #226   +/-   ##
=======================================
  Coverage   93.88%   93.88%           
=======================================
  Files          40       40           
  Lines        1259     1259           
=======================================
  Hits         1182     1182           
  Misses         77       77           
Files Coverage Δ
src/assertions.jl 100.00% <100.00%> (ø)
src/distributions.jl 91.66% <100.00%> (ø)
src/tableselection.jl 90.00% <100.00%> (ø)
src/transforms.jl 100.00% <100.00%> (ø)
src/transforms/dropextrema.jl 100.00% <100.00%> (ø)
src/transforms/eigenanalysis.jl 87.30% <100.00%> (ø)
src/transforms/functional.jl 89.79% <ø> (-0.21%) :arrow_down:
src/transforms/logratio.jl 100.00% <100.00%> (ø)
src/transforms/parallel.jl 95.06% <100.00%> (ø)
src/transforms/remainder.jl 100.00% <100.00%> (ø)
... and 2 more
CameronBieganek commented 9 months ago

Nitpick: This is better than using @assert, but it is more idiomatic to throw an ArgumentError than to throw an AssertionError.

juliohm commented 9 months ago

ArgumentError is for when the user misuses a function right? In the case of the parallel branches we thought that the user can safely create pipelines and that the error will only be encountered at runtime depending on the tables that are inserted.

We can reassess the type of error in the future.

Em ter., 7 de nov. de 2023 11:58, Cameron Bieganek @.***> escreveu:

Nitpick: This is better than using @assert, but it is more idiomatic to throw an ArgumentError than to throw an AssertionError.

— Reply to this email directly, view it on GitHub https://github.com/JuliaML/TableTransforms.jl/pull/226#issuecomment-1798777716, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQW3L53VI7EHOBIRLKQBLYDJEBJAVCNFSM6AAAAAA673PEKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJYG43TONZRGY . You are receiving this because you modified the open/close state.Message ID: @.***>