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

Allow member_id to be optional #54

Merged
merged 4 commits into from
Jul 15, 2021
Merged

Conversation

mgrover1
Copy link
Contributor

While working on a catalog with Dan Marsh, I ran into the following case b.e21.BWma1850.f09_g17.release-cesm2.1.3.c20200918 where the last part after g17. would be the member id, but cannot be split with the typical . separator. For now, I added a try except block where if it has an issue with the member id, it inserts None.

@mgrover1 mgrover1 requested a review from andersy005 June 22, 2021 13:55
@andersy005
Copy link
Member

andersy005 commented Jun 22, 2021

For now, I added a try except block where if it has an issue with the member id, it inserts None.

I'm curious... Does CESM have a naming convention for member_id? If so, could we replace info['member_id'] = int(info['case'].split('.')[-1]) with a more flexible, robust version?

@mgrover1
Copy link
Contributor Author

I think this is the problem Dan mentioned... perhaps removing the requirement that the member_id be an integer would be a better fix.

@andersy005
Copy link
Member

I think this is the problem Dan mentioned... perhaps removing the requirement that the member_id be an integer would be a better fix.

@mgrover1, 👍🏽 Could you run this on Dan's notebook to confirm that this works?

@mgrover1
Copy link
Contributor Author

@andersy005 I tested on this Dan's files, and it works. We need to make sure we are using the parse_cesm_history parser here too since technically these are history files (multiple variables), with a special case where there are multiple times as well (365 for each file)

Copy link
Member

@andersy005 andersy005 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, @mgrover1

@andersy005 andersy005 merged commit c600728 into ncar-xdev:main Jul 15, 2021
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