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

generalised unpack function after a model is solved #784

Merged
merged 10 commits into from
Aug 7, 2020

Conversation

MridulS
Copy link
Member

@MridulS MridulS commented Jul 30, 2020

fixes #457

Please ensure your pull request adheres to the following guidelines:

  • [N/A ] Tests for new functionality/models or Tests to reproduce the bug-fix in code.
  • Updated documentation of features that add new functionality.
  • Update CHANGELOG.md with major/minor changes.

@sbenthall
Copy link
Contributor

Is the idea that this is already covered by existing tests?

@MridulS
Copy link
Member Author

MridulS commented Jul 30, 2020

nope, this is supposed to fail for the time being. I want to discuss this in the call today.

Currently we use unpackcFunc and this PR introduces unpackFunc (notice the missing 'c') so we would need to update the code examples where we use model.unpackcFunc() with something like model.unpackFunc('cFunc') to have a generalised unpacking function. model.unpackFunc('cFunc') does look clunky so maybe we can have something like model.unpack('cFunc')

@llorracc
Copy link
Collaborator

llorracc commented Aug 6, 2020

Don't merge until the functionality of the old unpackcFunc is restored, deprecation notification is added

@MridulS MridulS changed the title [WIP] generalised unpack function after a model is solved generalised unpack function after a model is solved Aug 7, 2020
@MridulS MridulS merged commit 5a3ab2a into econ-ark:master Aug 7, 2020
@llorracc
Copy link
Collaborator

llorracc commented Aug 8, 2020

@MridulS

Thanks for fixing then merging this.

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.

Generalise unpackcFunc, make a general unpacking function
4 participants