Skip to content

remove tags from hazard classes#88

Merged
emanuel-schmid merged 6 commits intodevelopfrom
feature/remove_tag_hazard
Aug 25, 2023
Merged

remove tags from hazard classes#88
emanuel-schmid merged 6 commits intodevelopfrom
feature/remove_tag_hazard

Conversation

@emanuel-schmid
Copy link
Copy Markdown
Contributor

@emanuel-schmid emanuel-schmid commented Aug 8, 2023

Changes proposed in this PR:

  • Remove anything dealing with tags from the hazard classes of climada_petals.
    This is part of the overall climada refactoring to remove the Tag classes.

This PR fixes #

PR Author Checklist

PR Reviewer Checklist

@peanutfun peanutfun mentioned this pull request Aug 21, 2023
13 tasks
Copy link
Copy Markdown
Member

@peanutfun peanutfun left a comment

Choose a reason for hiding this comment

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

Looks good! I only suggest to improve the Changelog entry.

from climada.hazard.base import Hazard
from climada.hazard.centroids import Centroids
import climada.util.coordinates as u_coord
from climada.util.tag import Tag
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How did this work before? I don't see a climada/util/tag.py module in any of the recent releases

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

after release 3.3, hazard.tag and entity.tag have been merged

Comment on lines -280 to -283
yearrange=TARGET_YEARRANGE, yearrange_ref=REFERENCE_YEARRANGE,
gh_model=None, cl_model=None,
scenario='historical', scenario_ref='historical', soc='histsoc',
soc_ref='histsoc', fn_str_var=FN_STR_VAR, keep_dis_data=False):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious, that all of this information was only put into the tag 🤔

CHANGELOG.md Outdated
Comment on lines +20 to +21
- several adaptations have been necessary because of the `Tag` class being removed from the CLIMADA core package:
[#88](https://github.com/CLIMADA-project/climada_petals/pull/88)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suggest to put this into the "Removed" category:

### Removed

- `tag` attribute from hazard classes [#88](https://github.com/CLIMADA-project/climada_petals/pull/88)

@emanuel-schmid
Copy link
Copy Markdown
Contributor Author

🙌 thanks for the review!

@emanuel-schmid emanuel-schmid merged commit ccb0314 into develop Aug 25, 2023
@emanuel-schmid emanuel-schmid deleted the feature/remove_tag_hazard branch August 25, 2023 14:03
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