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.

Sign up for GitHub

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

Jump to bottom

rip out formula parsing and replace it with something sane #54

Merged
merged 20 commits into from
Apr 22, 2018
Merged

rip out formula parsing and replace it with something sane #54

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
38a5134
drop in new parsing code
kleinschmidt Apr 16, 2018
6ed79ba
use typemax(Int) instead of Inf
kleinschmidt Apr 16, 2018
2c72081
fix onesided formulae
kleinschmidt Apr 16, 2018
969c5bc
more tests are broken
kleinschmidt Apr 16, 2018
b162f95
guess debug macro isn't exported
kleinschmidt Apr 16, 2018
94acf5b
get rid of debugging code
kleinschmidt Apr 16, 2018
ff277c8
using argcheck in formula: cut the crap and indent
kleinschmidt Apr 16, 2018
0f88bfa
forgot a Compat.findfirst
kleinschmidt Apr 16, 2018
5918628
remove argcheck dependency
kleinschmidt Apr 18, 2018
d2e0ba0
updating tests
kleinschmidt Apr 18, 2018
99a411b
actually just use Terms to get functional version of formula
kleinschmidt Apr 18, 2018
e28da1b
rewrite rule for interactions with number terms
kleinschmidt Apr 18, 2018
c96f6e4
make revise-friendly, deleteat ex.args, missing ||
kleinschmidt Apr 18, 2018
e379f25
typo
kleinschmidt Apr 18, 2018
cca0e6f
docstrings for new rewrites and update tests
kleinschmidt Apr 18, 2018
0090e78
remove dead code
kleinschmidt Apr 18, 2018
8da82bc
add (deprecated) 2-arg Formula constructor, and tests
kleinschmidt Apr 19, 2018
643bc8c
use different warning tests depending on the version
kleinschmidt Apr 19, 2018
bc8c314
use @ static for version check
kleinschmidt Apr 19, 2018
f1b98c2
check version against 0.7.0-DEV.2988
kleinschmidt Apr 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions REQUIRE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ julia 0.6
DataFrames 0.11.5
StatsBase 0.20.1
Compat 0.63
ArgCheck
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really prefer we not add a dependency on ArgCheck, since @argcheck thing "ahhh" is identical to thing || throw(ArgumentError("ahhh")), and the latter is both clearer (IMO) and will provide a cleaner backtrace (nothing about inlined macro expansion from @argcheck).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more of a habit than anything for me so I'm fine to do the checks manually. At this point that wouldn't add too much boilerplate. Your objection is just that argcheck isn't strictly necessary here, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. And I always favor fewer dependencies when possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT you removed all uses of @argcheck but still added the dependency.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I caught this right after I tagged a release so I pushed another commit on master to remove it. Thanks attobot for showing the requires diff.

16 changes: 7 additions & 9 deletions src/StatsModels.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ using DataFrames
using StatsBase
using Compat.SparseArrays
using Compat.LinearAlgebra
using Compat: @debug, @warn

export @formula,
Formula,
Expand All @@ -23,14 +24,11 @@ export @formula,
dropterm,
setcontrasts!

map(include,
[
"contrasts.jl",
"formula.jl",
"modelframe.jl",
"modelmatrix.jl",
"statsmodel.jl"
])

include("contrasts.jl")
include("formula.jl")
include("modelframe.jl")
include("modelmatrix.jl")
include("statsmodel.jl")
include("deprecated.jl")

end # module StatsModels
6 changes: 6 additions & 0 deletions src/deprecated.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
function Formula(lhs, rhs)
Base.depwarn("Formula(lhs, rhs) is deprecated. Use @eval(@formula($lhs ~ $rhs)) if " *
"parsing is required, or Formula(ex_orig, ex, lhs, rhs) if not",
:Formula)
Formula(:(), :($lhs ~ $rhs), lhs, rhs)
end
Loading