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

Rewind the stream on get_stream #35

Closed
wants to merge 1 commit into from

Conversation

alifbe
Copy link

@alifbe alifbe commented Oct 7, 2022

When user passed file object instead of filename, the stream need to be re-winded so that it start from the beginning

@eivindjahren
Copy link
Collaborator

eivindjahren commented Oct 7, 2022

Thanks for your contribution!

I think having the stream pointing at where you want ecl_data_io.read and ecl_data_io.write to start reading/writing is up to the call-site/user. Insisting on starting from the beginning of the stream makes ecl_data_io inflexible and is a breaking change.

See for instance yaml.load, it behaves in the same way.

We can discuss this further if you still feel this change needs to be made or perhaps I have missed something.

@alifbe
Copy link
Author

alifbe commented Oct 7, 2022

@eivindjahren Thanks for the feedback.

I see your point about the flexibility. The reason for the PR is related to the other PR in xtgeo equinor/xtgeo#807

I have made that grid will automatically update ACTNUM when trying to read INIT file that has PORV information. It works fine when working with file path. But the unit test is designed with the stream directly, hence it failed the test (due to the stream exhausted after trying to find PORV). Hence I made this PR to rewind it whenever use tried to get_stream.

Do you have any recommendation on how it should be designed?

@eivindjahren
Copy link
Collaborator

@alifbe I saw your xtgeo issue and I am interested in seeing how intersect is behaving differently than eclipse when it comes to init files.

I will try to help out however I can.

Would it work for you to rewind the stream before calling ecl_data_io.read in the test?

@alifbe
Copy link
Author

alifbe commented Oct 9, 2022

@eivindjahren Yes, it works. I added a new commit to rewind the stream if user didn't passed a string or path and reset the position afterward.

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.

None yet

2 participants