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

code to enforce immutable polytopes. #61

Closed

Conversation

navidmokh
Copy link

Just a potential idea for the future.

TODO add similar protection for Regions.

@johnyf
Copy link
Member

johnyf commented Oct 8, 2020

Immutability of the classes polytope.polytope.Polytope and polytope.polytope.Region has been discussed in #6 and was decided to keep these classes mutable. For example, commit 479e9ce on branch overhaul also makes A and b properties, but with setters.

@johnyf
Copy link
Member

johnyf commented Oct 8, 2020

There are errors on Travis CI due to this change.

@navidmokh
Copy link
Author

navidmokh commented Oct 8, 2020

That overhaul commit does exactly what I had written originally in the setter. I am fully in favor of that change, hopefully it can be integrated. In the meantime, I am clearing the cached results myself every-time with a wrapper function. My only concern with allowing mutability of polytopes is the impact that decision would have within the region class and the memoization occuring there. Instead we could update the caching to confirm a hash of the A & b numpy arrays. This way, after checking None, we verify that the cached data still corresponds with the number and shapes of the polytopes in list_poly.

@navidmokh navidmokh closed this Oct 8, 2020
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.

2 participants