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

QPLIB format parser #250

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

QPLIB format parser #250

wants to merge 6 commits into from

Conversation

Zengor
Copy link
Contributor

@Zengor Zengor commented Dec 24, 2024

Adds the ability to read QPLIB files to create OMMX instances.

Some consideration has been given to adding the ability to output QPLIB format in the future but some aspects of it are still open.

@Zengor Zengor marked this pull request as ready for review January 8, 2025 07:32
@termoshtt termoshtt self-requested a review January 10, 2025 02:44
Comment on lines +39 to +45
// a little hack to allow us to add the line numbers
// as context to errors generated in `FromStr` impls
#[error("{inner} (at line {line_num})")]
WithLine {
line_num: usize,
inner: Box<QplibParseError>,
},
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to convert WithLine as ParseError type, and other entries as separated enum, e.g. ParseErrorReason.

#[error(transparent)]
ParseFloat(#[from] std::num::ParseFloatError),
#[error(transparent)]
Io(#[from] std::io::Error),
Copy link
Member

Choose a reason for hiding this comment

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

IO error is not a parse error. Since std::io::Error does not contains which file is tried to opened, returning this error is not user-friendly. We should rather return anyhow::Result from function which may returns IO error with anyhow::Context to tell which file is opened.

For the actual parse errors like invalid var types, creating this ParseError and returned within anyhow::Error. Users can get the error as ParseError via downcast https://docs.rs/anyhow/latest/anyhow/struct.Error.html#method.downcast

Copy link
Member

Choose a reason for hiding this comment

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

Similar to MPS, we want to create some cases which is invariant via the round-trip ommx.v1.Instance -> QPLIB format -> ommx.v1.Instance. Do you have any idea especially for ID mapping.

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