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.

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

Style improvements for the MOI wrapper #115

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

odow
Copy link
Contributor

@odow odow commented Jun 29, 2022

This PR starts to tidy up the MOI wrapper.

I've tried to keep this fairly surface level. I haven't re-arranged any code blocks to keep the diff easier to read. I'll do that in another PR.

@odow odow changed the title Remove acronyms in the MOI wrapper Style improvements for the MOI wrapper Jun 29, 2022
using LinearAlgebra: rmul!
import LinearAlgebra
import MathOptInterface
import SparseArrays
Copy link
Collaborator

@blegat blegat Jun 30, 2022

Choose a reason for hiding this comment

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

I prefer importing stdlib first, maybe this could be in the style guide. In general, I even load any dependency first so I do some sort of topological sort

lower(::MOI.Nonpositives, i::Int) = -Inf
upper(::MOI.Zeros, i::Int) = 0.0
upper(::MOI.Nonnegatives, i::Int) = Inf
upper(::MOI.Nonpositives, i::Int) = 0.0

# TODO: just use ∈ on 0.7 (allocates on 0.6):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function can be removed according to this comment

@matbesancon
Copy link
Contributor

what was missing on this one?

@blegat
Copy link
Collaborator

blegat commented Aug 1, 2023

I think a rebase should be enough my comments were pretty minor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants