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

Fix template + update_attr bug #405

Merged
merged 10 commits into from
May 15, 2024
Merged

Fix template + update_attr bug #405

merged 10 commits into from
May 15, 2024

Conversation

juliettelavoie
Copy link
Contributor

@juliettelavoie juliettelavoie commented May 15, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • (If applicable) Documentation has been added / updated (for bug fixes / features).
  • (If applicable) Tests have been added.
  • This PR does not seem to break the templates.
  • CHANGES.rst has been updated (with summary of main changes).
    • Link to issue (:issue:number) and pull request (:pull:number) has been added.

What kind of change does this PR introduce?

  • A few fixes to the templates due to changes in xscen and xarray
  • Fix update_attr bug #404, I don't know why this worked in the past but not now. From my understanding there were 2 problems:
  1. We iterate over others and assign it to others. This breaks the iteration.
  2. In the first instance, we pass a dict of dataset to create a dict of str (l.137). Then, when we iterate over others in l.147, it is no longuer a dict of dataset, we can't get to attrs.
    Let me know if I misunderstood something.

Does this PR introduce a breaking change?

i don't think so.

Other information:

@@ -133,20 +133,23 @@ def update_attr(
others = others or []
# .strip(' .') removes trailing and leading whitespaces and dots
if attr in ds.attrs:
others = {

others_attrs = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Woups, c'était ben niaiseux mon truc. haha
Le code n'a été testé qu'avec des dataset d'une seule variable, je pense que c'est pour ça qu'on a pas vu ça avant.

CHANGES.rst Outdated
@@ -2,6 +2,15 @@
Changelog
=========

v0.10.0 Unreleased
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
v0.10.0 Unreleased
v0.9.1 (unreleased)

@juliettelavoie juliettelavoie merged commit 91a8500 into main May 15, 2024
24 checks passed
@juliettelavoie juliettelavoie deleted the fix-template branch May 15, 2024 20:43
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.

update_attr bug
2 participants