-
Notifications
You must be signed in to change notification settings - Fork 2
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
Increment Docs + Add Templates #8
Conversation
PseudoBooleanFunction | ||
reduce_degree | ||
Δ | ||
``` |
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.
We should not export nor document in the main doc the internet function that users won't use, only the one we believe there is a chance users will find useful. They should still have docstrings though.
The main issue is keeping compatibility and deprecating many functions.
Another good practice is, if a function, is exported there should be a version of it that does not use special characters, some users have trouble with them, this would be an unnecessary barrier.
T::Type{<:S}, | ||
model::MOI.ModelLike, | ||
optimizer::Union{Nothing, MOI.AbstractOptimizer}=nothing; | ||
ϵ::S=zero(S) |
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.
if it is a user exported function it is dangerous to have kwargs that are special chars, some users will have trouble with that.
@@ -1,4 +1,4 @@ | |||
module VarMap | |||
module VirtualMapping | |||
|
|||
export coefficient, coefficients, offset, isslack, source, target, name | |||
export isempty, length, iterate |
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.
no need to export base functions
@@ -25,24 +24,23 @@ const CI = MOI.ConstraintIndex | |||
const ∅ = Set{VI}() |
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.
careful here. This is always the same set.
if you use this to initialize sets they will get mixed up.
@@ -0,0 +1,159 @@ | |||
# Examples |
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.
Ideally, we should have one file per example.
Also it is always better do it with Literate.jl with a base .jl file so that anyone can just include
the file.
JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6" | ||
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" | ||
MathOptInterface = "b8f27783-ece8-5eb3-8dc8-9495eed66fee" | ||
Parsers = "69de0a69-1ddd-5017-9359-2bf0b02dc9f0" | ||
PyCall = "438e738f-606a-5dbb-bf0a-cddfbfd45ab0" | ||
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" | ||
RecipesBase = "3cdcf5f2-1ef4-517c-9805-6587b60abb01" |
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.
add compat for all deps.
Review which packages are required for the main code e which should only be on tests.
Add a few examples and some placeholder text