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

tickets/DM-20128: Create tests #99

Merged
merged 23 commits into from
Jun 24, 2019
Merged

tickets/DM-20128: Create tests #99

merged 23 commits into from
Jun 24, 2019

Conversation

fred3m
Copy link
Collaborator

@fred3m fred3m commented Jun 22, 2019

This ticket creates tests for the bulk of scarlet classes and functions.

fred3m added 20 commits June 21, 2019 22:53
While writing tests I found that the `BoundingBox` API was clunky and
supported features that were inconsistent. The new implementation is
cleaner and uses a shortened `Box` class name. The `resize` method
was also removed, because it didn't add anything to the functionality
of trim.
Generating a PSF image for given function in a given shape is
common enough that a convenience method has been added to generate
the x, y pixel grid for the shape and create a PSF image using the
function parameters.
While creating the tests a few inconsistencies/sloppy coding were
fixed:
- `Scene.psfs` is now always 3D, even the `target_psf` for the
  model scene. This makes the API more uniform for `Scene`
  and its inherited classes.
- PSF matching and convolutions now use real FFTs. This has the
  advantage that it runs faster, fixes a potential bug in the
  PSF matching code, and is more similar to the `scipy.signal.convolve`
  method.
In addition to creating the units tests for the classes in component.py,
the commit also fixes the `Prior` API to use the form `sed_grad` and
`morph_grad` (instead of `grad_sed`, ...) to match the attributes of
`Component`.
This also includes calling `update` at the end of
`PointSource.__init__` and `ExtendedSource.__init__`.
@fred3m fred3m requested a review from pmelchior June 22, 2019 03:04
@fred3m
Copy link
Collaborator Author

fred3m commented Jun 22, 2019

This ticket contains tests for the bulk of scarlet. I would still like to come up with a good test for initializing ExtendedSource and ensuring that the autograd functions are working as they should, but this dramatically increases our test coverage.

I decided not to include tests for building the docs since that looks a bit complicated and we have more pressing changes to implement. Once those are pushed to master we can revisit building the docs on Travis and adding a flake8 test to ensure a consistent coding style.

@pmelchior
Copy link
Owner

Before looking at the code, that all checks have failed on this branch is not a good sign...

docs/install.rst Outdated Show resolved Hide resolved
@fred3m
Copy link
Collaborator Author

fred3m commented Jun 24, 2019

Before looking at the code, that all checks have failed on this branch is not a good sign...

This is why I said that proxmin had to be merged first. All tests that depended on the correct proxmin behavior (eg. inline updates) fail. Now that proxmin has been merged I changed the version of proxmin required to install scarlet so everything should build, since it builds on my local computer.

@pmelchior pmelchior merged commit fec0db8 into master Jun 24, 2019
@pmelchior pmelchior deleted the tickets/DM-20128 branch June 24, 2019 04:06
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