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

Add support for python dimacs reader in memory #135

Open
mtreinish opened this issue May 14, 2021 · 2 comments
Open

Add support for python dimacs reader in memory #135

mtreinish opened this issue May 14, 2021 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@mtreinish
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Currently the read_dimacs_file method exposed through the python api requires a file path and works by opening the file. This puts an unnecessary constraint when using the api to write out a file to disk especially if the string is constructed in python this just seems unecessary.

Describe the solution you'd like

The dimacs parser being used supports passing in a dimacs file as an std::istream: https://lorina.readthedocs.io/en/latest/dimacs.html#_CPPv4N6lorina11read_dimacsERNSt7istreamERK13dimacs_readerP17diagnostic_engine

we can expose this via the python interface as a file like object (not sure how that connects with pybind11 but there should be a way). Ideally the interface would take either a file like object like a file, BytesIO,or a raw string we can pass to this.

Describe alternatives you've considered

The other alternative is just in pure python, basically adds a method that takes a stream or string and dumps it to a temp file and then we pass the temp file path to the existing read_dimacs, but doing it this way would just be shifting the current workaround from external to tweedledum to internal.

Additional context

@mtreinish mtreinish added the enhancement New feature or request label May 14, 2021
@boschmitt boschmitt self-assigned this May 14, 2021
@boschmitt boschmitt added this to the v1.2 milestone Jun 29, 2021
@boschmitt boschmitt added the good first issue Good for newcomers label Jun 29, 2021
@boschmitt boschmitt removed their assignment Jun 29, 2021
@boschmitt
Copy link
Owner

Binding to istream seems to be complicated. I think that for the purposes of
unit testing it should be enough to accept a string which contains the whole
DIMACS. (I expect that the most common usage would actually be for passing
the a file.)

I will work on this later, unless anyone else want to give it a try as a firs contribution
to tweedledum.

@1ucian0
Copy link
Contributor

1ucian0 commented May 24, 2022

+1 to support istream. It would be much easier in a demo to define the content of the file in a your jupyter notebook and then using it directly as StringIO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants