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

automate the addition of default cf attributes? #146

Open
jerabaul29 opened this issue Nov 15, 2024 · 3 comments · May be fixed by #153
Open

automate the addition of default cf attributes? #146

jerabaul29 opened this issue Nov 15, 2024 · 3 comments · May be fixed by #153

Comments

@jerabaul29
Copy link
Collaborator

Should we automate the addition of default cf attributes when these are missing? Or should this be something the user has to call in manually? I.e. the addition of the attributes as done by:

ds = ds.traj.assign_cf_attrs()

Could look for the existence of the requested attributes in the class __init__ when a Traj object is created for example, and call this automatically if the attributes are not there? Any thoughts about this @gauteh @knutfrode ? :)

@knutfrode
Copy link
Contributor

Maybe this could be added by default if we make a ds.traj.to_netcdf() (but still with possibility to skip)

@jerabaul29
Copy link
Collaborator Author

Agree that would be a good place to have such a call. Maybe having this call overwrite previous attributes would be good, so that also in case the dataset has been updated / sliced, the attributes are correct when writing to disk?

Would it make sense to call this actually quite often, for example both at opening if the attributes are missing (so the attributes are always present, they are convenient to always have available), and always even if present from before at writing to disk (so that we never write outdated attributes)? :)

@gauteh
Copy link
Member

gauteh commented Nov 15, 2024

I don't think we should modify the dataset in init without any action by the user. In theory it needs to be updated in a lot of cases, seltime, gridtime, isel(trajectory=0) (which we don't control). Maybe in to_netcdf is a good place. I was also wondering if this should be normalize() method, which also fixes other inconsistencies (like missing role attributes etc).

@jerabaul29 jerabaul29 linked a pull request Nov 28, 2024 that will close this issue
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 a pull request may close this issue.

3 participants