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.

Sign up for GitHub

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

Jump to bottom

Implement gather_dimensions for reducing no dimensions and preserving… #20

Merged
merged 4 commits into from
Jul 24, 2023
Merged

Implement gather_dimensions for reducing no dimensions and preserving… #20

merged 4 commits into from
Jul 24, 2023

Conversation

tennlee
Copy link
Collaborator

@tennlee tennlee commented Jun 22, 2023

… all dimensions

@tennlee tennlee linked an issue Jun 22, 2023 that may be closed by this pull request
Clarify and refine dimension gathering rules #19
Closed
nicholasloveday
nicholasloveday requested changes Jul 19, 2023
View reviewed changes
Copy link
Collaborator

@nicholasloveday nicholasloveday left a comment

Choose a reason for hiding this comment

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

It behaved as expected when I tested it out on some data.

  • Consider if mse needs a test where preserve_dims= is called.
  • Consider if DimensionError should specify the problematic dim. We can add an issue if you think this is worth doing.

tests/scores/test_utils.py Outdated Show resolved Hide resolved
tests/scores/test_utils.py Outdated Show resolved Hide resolved
tests/scores/test_utils.py Outdated Show resolved Hide resolved
@tennlee
Standardise set handling logic
b536dac 
Refactor tests for clarity
Add additional clear basic tests for reduce and preserve
@nicholasloveday
Copy link
Collaborator

This looks good to me.

A note for @aidanjgriffiths 's pull request , is that the type that utils.gather_dimensions now returns is a set.

@tennlee tennlee merged commit 3fee1b9 into main Jul 24, 2023
tennlee added a commit that referenced this pull request Aug 21, 2023
@aidanjgriffiths @tennlee
#28 align develop branch with latest main (#29)
ec47cc1 
* chore: swap setuptools for hatchling pkg build backend (#7)

Co-authored-by: agriffit <aidan.griffiths@bom.gov.au>

* doc: Update example in README (#15)

* Implement gather_dimensions for reducing no dimensions and preserving… (#20)

* Implement gather_dimensions for reducing no dimensions and preserving all dimensions
* Implement more checks in gather_dimensions
* Distinguish the error messages for preserving a nonexistent dimension from reducing a nonexistent dimension
* Standardise set handling logic
* Refactor tests for clarity
* Add additional clear basic tests for reduce and preserve

* PyTest action (#25)

* PyTest action
* Rename action to "Unit Tests"
* Use python version correctly in action
* Use Python version in job name

---------

Signed-off-by: tennlee <134973832+tennlee@users.noreply.github.com>

---------

Signed-off-by: tennlee <134973832+tennlee@users.noreply.github.com>
Co-authored-by: agriffit <aidan.griffiths@bom.gov.au>
Co-authored-by: tennlee <134973832+tennlee@users.noreply.github.com>
@tennlee tennlee mentioned this pull request Aug 21, 2023
Closed
@tennlee tennlee deleted the 19-reduce_dims_empty branch March 29, 2024 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicholasloveday nicholasloveday nicholasloveday approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clarify and refine dimension gathering rules
2 participants
@tennlee @nicholasloveday