Skip to content

get stress table output from OVERVIEW.OUT #47

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

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

Conversation

jeevakir
Copy link

@jeevakir jeevakir commented Dec 9, 2024

added a function to get stage-wise stress output from OVERVIEW.OUT. The function handles only the first run. Will add functionality to create table for all runs with run number as index. Please make changes if anything is needed. thanks for the package.

@jeevakir jeevakir marked this pull request as ready for review December 9, 2024 11:06
@jeevakir jeevakir closed this Dec 12, 2024
@jeevakir jeevakir reopened this Dec 12, 2024
corrected a small mistake where i had if condition wrong.
@daquinterop
Copy link
Owner

Hi! thanks for your contribution. I'm reviewing the request and the changes in the code is causing some of the tests to fail. I'll modify what is causing the issue and then I'll merge it. Also, instead of saving the output in _ouput['OVERVIEW'], I'll create a new element in the output with the name of "stress". This is due to the fact that the overview file not only contains stress tables, it also contains other type of information that may be useful for other users.

@daquinterop
Copy link
Owner

After reviewing I made a couple changes. It seems Perennial forages don't generate the overview file, that was causing some issues. Now, I'm adding the stress into a new DSSAT.stress_table attribute. The OVERVIEW file is saved in the output attribute as the lines of the raw file. However, I see that there is some problems with the stress table, it seems that the Dev stage column is not being created is it's supposed to be. You can check what I mean in one of the tests, maybe the Maize test is the best example to work with.

I just pushed the changes I made, and as soon as we make sure the new function correctly creates the stress tables for all crops, then I'll proceed to merge and update the PyPI package.

Thanks again for your contribution.

@jeevakir
Copy link
Author

jeevakir commented Dec 24, 2024

Hi, Thank you for the library again. Your changes makes more sense as it will be easier to understand than naming it as _output['OVERVIEW'] . Sorry for raising an incomplete pull request without running tests. I just created it quickly as a favor. I will test it from my side. If possible i will try to expand the _get_overview_stress() to include stress tables for all runs instead of just the first run. Once again, my apologies for the improper pull request. Please let me know if there is a specific way to go about it for indexing the tables.

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