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

Fix Gaussian Shape docs #209

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Fix Gaussian Shape docs #209

wants to merge 3 commits into from

Conversation

sjperkins
Copy link
Member

Fixes #207

  • Tests added / passed

    $ py.test -v -s africanus

    If the pep8 tests fail, the quickest way to correct
    this is to run autopep8 and then flake8 and
    pycodestyle to fix the remaining issues.

    $ pip install -U autopep8 flake8 pycodestyle
$ autopep8 -r -i africanus
$ flake8 africanus
$ pycodestyle africanus

  • Fully documented, including HISTORY.rst for all changes
    and one of the docs/*-api.rst files for new API

    To build the docs locally:

    pip install -r requirements.readthedocs.txt
cd docs
READTHEDOCS=True make html

@sjperkins
Copy link
Member Author

Ignore my previous comment, I need to put the scaling factor in.

@sjperkins sjperkins marked this pull request as draft July 7, 2020 11:40
@sjperkins
Copy link
Member Author

Documentation now looks like:

Screenshot from 2020-07-07 14-00-27

Do you have a reference for the emaj/emin/position angle conventions?

@landmanbester
Copy link
Collaborator

Sorry, I think I pointed at the wrong line. In the code, v1 is defined as

https://github.com/ska-sa/codex-africanus/blob/d4e4c01de1029e418f03c5fb096829a3202986b2/africanus/model/shape/gaussian_shape.py#L45

So there is a plus instead of the minus as indicated in the docs. Also, the effect of multiplying u1 by the ratio r is simply to replace emaj with emin so I am not sure what the point of r really is. Is this parametrisation correct? I would have to work out what the general parameterization of a 2D Gaussian is in Fourier space, don't know it off the top of my head. It seems to be correct for

.

Also, we should document the fact that emaj and emin refer to the FWHM of the Gaussian not the standard deviation to clarify the factor of

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@landmanbester landmanbester Awaiting requested review from landmanbester

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

Successfully merging this pull request may close these issues.

Inconsistent documentation for Gaussian shape function
2 participants
@sjperkins @landmanbester