Skip to content

Draft enhance 01-marketsplit#15

Merged
ksk-jij merged 19 commits intomainfrom
01-marketsplit
Oct 2, 2025
Merged

Draft enhance 01-marketsplit#15
ksk-jij merged 19 commits intomainfrom
01-marketsplit

Conversation

@ahao27
Copy link
Collaborator

@ahao27 ahao27 commented Aug 19, 2025

Draft enhance the 01-marketsplit

@ahao27
Copy link
Collaborator Author

ahao27 commented Sep 17, 2025

Fix the #3 (comment)

The directory name should be model instead of Model if there is no big reason.

Fix the directory name to models.

File name as well. OMMX_create.py should be ommx_create.py.

Fix to the ommx_create.py.

from typing import xxx is not needed if the python version is 3.10. Use like dict instead of typing.Dict.

Remove the typing.

Sets of code being able to use the different file should be stored in somewhere and use it instead of writing the same thing. (Probably nice to create common directory under 01-marketsplit and put utility functions and classes.)

I keep the structure same as other problems.

Why does only dat_reader.py have class? Like Why not sol_reader.py?

Change the dat_reader to the function.

Why are only ommx files compressed?

Removed the ommx files.

Please write how to create ommx file in README.md. (Probably after changing the current directory to the target directory and run uv run python OMMX_create.py.)

Already in the README.md. Thank for adding.

Apply black formatter.

Applied.

@ahao27 ahao27 requested a review from ksk-jij September 17, 2025 05:48
Copy link
Collaborator

@ksk-jij ksk-jij left a comment

Choose a reason for hiding this comment

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

Thank you for the update! Looks good to me! However, I put one question I'd like to know!

@ksk-jij
Copy link
Collaborator

ksk-jij commented Oct 2, 2025

Close #3 once this PR is merged

@ksk-jij ksk-jij merged commit 7b62aa1 into main Oct 2, 2025
9 checks passed
@ksk-jij ksk-jij deleted the 01-marketsplit branch October 2, 2025 05:42
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