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

MNT: Rename description to descriptor in filename variable #313

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

greglucas
Copy link
Collaborator

Change Summary

Overview

Change "description" to "descriptor" for the filenames.

Additionally, descriptor is not optional anymore, so set a default of sci in the field. Or do we want to just force everyone to input this when calling write_cdf()?

@@ -41,7 +41,7 @@ def calc_start_time(shcoarse_time: int):

def write_cdf(
data: xr.Dataset,
description: str = "",
descriptor: str = "sci",
Copy link
Contributor

Choose a reason for hiding this comment

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

To you question, I think we shouldn't make this optional input now since this field is pretty generic and can be so different based on individual instrument.

Or if we put it in dataset.attrs, then we can abstract from that too. But could be up for discussion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or if we put it in dataset.attrs, then we can abstract from that too. But could be up for discussion.

I like this idea, but I am also not sure how it would look because the instrument already has a "Descriptor" which is: "SWE>Solar Wind Electron" which isn't what we want unfortunately.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, I just made it non-optional now. We can look into the attrs idea later.

Copy link
Contributor

Choose a reason for hiding this comment

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

that sounds good. I can do that in follow-up PR.

@greglucas greglucas merged commit aa30a30 into IMAP-Science-Operations-Center:dev Jan 12, 2024
17 checks passed
@greglucas greglucas deleted the descriptor branch January 12, 2024 22:05
laspsandoval pushed a commit to laspsandoval/imap_processing that referenced this pull request Apr 2, 2024
…descriptor

MNT: Rename description to descriptor in filename variable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants