Skip to content

Navigation Menu

Sign in
Appearance settings
Sign up
Appearance settings

Rename SpatialSkill to GriddedSkill - #315

#315
Merged
ecomodeller merged 24 commits into
mainDHI/modelskill:mainfrom
gridskillDHI/modelskill:gridskillCopy head branch name to clipboard
Dec 11, 2023
Merged

Rename SpatialSkill to GriddedSkill#315
ecomodeller merged 24 commits into
mainDHI/modelskill:mainfrom
gridskillDHI/modelskill:gridskillCopy head branch name to clipboard

Conversation

@ecomodeller

@ecomodeller ecomodeller commented Dec 7, 2023

Copy link
Copy Markdown
Member
  • Rename method cc.spatial_skill() to cc.gridded_skill()
  • Rename class SpatialSkill to SkillGrid
  • Create new class SkillGridArray (for a single metric and plotting)
  • Move plotting to SkillGridArray (and remove field argument)
  • Allow getitem and getattr in SkillGrid -> SkillGridArray

@jsmariegaard

jsmariegaard commented Dec 7, 2023

Copy link
Copy Markdown
Member

grid_skill or gridded_skill 🤔? or skill_grid?

Comment thread modelskill/comparison/_collection.py Outdated
n_min=None,
**kwargs,
):
warnings.warn("grid_skill is deprecated, use grid_skill instead", FutureWarning)

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.

🤔(grid_skill is dead, long live grid_skill)

@jsmariegaard jsmariegaard changed the title Rename SpatiaSkill to GridSkill Rename SpatialSkill to GridSkill Dec 8, 2023
@jsmariegaard jsmariegaard changed the title Rename SpatialSkill to GridSkill Rename SpatialSkill to GriddedSkill Dec 9, 2023
@jsmariegaard
jsmariegaard self-requested a review December 11, 2023 09:58

@jsmariegaard jsmariegaard left a comment

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 am happy with this.

@ecomodeller should we merge?

@jsmariegaard

Copy link
Copy Markdown
Member

Oops - mypy error - but apart from this I am happy

@ecomodeller
ecomodeller merged commit 55883fd into main Dec 11, 2023
@ecomodeller
ecomodeller deleted the gridskill branch December 11, 2023 10:15
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.