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

test: update test suite for use with jax-galsim #17

Merged
merged 1 commit into from
Jul 28, 2024
Merged

test: update test suite for use with jax-galsim #17

merged 1 commit into from
Jul 28, 2024

Conversation

beckermr
Copy link
Contributor

@beckermr beckermr commented Oct 3, 2023 •
edited
Loading

This PR updates the coord test suite so that it can be used with jax-galsim. The updates include

  • moving an import to avoid breaking the way jax-galsim replaces coord during testing
  • accounting for slightly different errors raised by jax-galsim
  • accounting for slightly different numerics raised by jax-galsim
  • splitting up some of the tests so that it is easier to figure out breakages

@beckermr beckermr changed the base branch from releases/1.3 to main July 25, 2024 20:33
@beckermr beckermr changed the title TST do not reimport coord test: do not reimport coord Jul 26, 2024
@beckermr beckermr marked this pull request as ready for review July 26, 2024 17:41
@beckermr
Copy link
Contributor Author

OK @rmjarvis this is ready for review. I made a change to the codecov.yml config to ignore the tests. However, there is an issue with the upload in the CI. They want you to make a token and use their GHA thing I think. I had to do this elsewhere. Only you have the access to do this, so I think it'll have to wait for another PR.

@rmjarvis
Copy link
Collaborator

Yeah, I've switch to that style in a few places too.

@rmjarvis
Copy link
Collaborator

OK, you should be able to rebase to main now and get correct coverage reports.

@beckermr
Copy link
Contributor Author

Alrighty then! @rmjarvis this one is really ready for review now!

rmjarvis
rmjarvis requested changes Jul 26, 2024
View reviewed changes
Copy link
Collaborator

@rmjarvis rmjarvis left a comment

Choose a reason for hiding this comment

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

I have questions.

.codecov.yml Outdated Show resolved Hide resolved
coord/_version.py Outdated Show resolved Hide resolved
tests/helper_util.py Show resolved Hide resolved
tests/test_angle.py Outdated Show resolved Hide resolved
tests/test_angleunit.py Outdated Show resolved Hide resolved
beckermr
beckermr commented Jul 26, 2024
View reviewed changes
Copy link
Contributor Author

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

changes for CR.

.codecov.yml Outdated Show resolved Hide resolved
.codecov.yml Outdated Show resolved Hide resolved
coord/_version.py Outdated Show resolved Hide resolved
tests/test_angle.py Outdated Show resolved Hide resolved
tests/test_angleunit.py Outdated Show resolved Hide resolved
beckermr
beckermr commented Jul 26, 2024
View reviewed changes
tests/test_angleunit.py Outdated Show resolved Hide resolved
@rmjarvis
Copy link
Collaborator

Also, you'll need to properly rebase this. GitHub is complaining.

@beckermr beckermr changed the title test: do not reimport coord test: update test suite for use with jax-galsim Jul 26, 2024
@beckermr beckermr requested a review from rmjarvis July 26, 2024 20:45
@beckermr
Copy link
Contributor Author

ok done with all of the comments @rmjarvis! We should squash and merge the history here if this PR is approved.

@beckermr
test: fix tests to not reimport coord and work with jax-galsim
ed168fe 
Update helper_util.py

TST move assert for raises to other tests

TST move assert for raises to other tests

TST move assert for raises to other tests

TST use absolute tolerance to compare to zero

TST use absolute tolerance to compare to zero

TST use absolute tolerance to compare to zero

TST use absolute tolerance to compare to zero

TST use absolute tolerance to compare to zero

raises different error

raises different error

fix: adjust coverage to exclude tests

fix: changes for CR

fix: this is a unit

Update tests/helper_util.py

Apply suggestions from code review

Remove docs on main

build docs

TST do not reimport coord

TST move assert for raises to other tests

TST move assert for raises to other tests

TST move assert for raises to other tests

TST use absolute tolerance to compare to zero

TST use absolute tolerance to compare to zero

TST use absolute tolerance to compare to zero

TST use absolute tolerance to compare to zero

fix: remove docs again

fix: adjust coverage to exclude tests

fix: changes for CR

Update tests/helper_util.py

Apply suggestions from code review
@beckermr
Copy link
Contributor Author

OK @rmjarvis this is all rebased into a single commit.

@rmjarvis rmjarvis merged commit f812084 into main Jul 28, 2024
10 checks passed
@rmjarvis rmjarvis deleted the fix-tests-jax-galsim branch July 28, 2024 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmjarvis rmjarvis rmjarvis approved these changes

Assignees

@rmjarvis rmjarvis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@beckermr @rmjarvis