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

Add VII interpolator. #22

Merged
merged 10 commits into from
Jun 5, 2020
Merged

Add VII interpolator. #22

merged 10 commits into from
Jun 5, 2020

Conversation

sjoro
Copy link
Contributor

@sjoro sjoro commented May 15, 2020

Add VII interpolator. A first prototype interpolation is provided and it's purpose is to navigate the simulated test data which most likely still has some things to be ironed out. The code may have to be revisited later on.

  • Tests added
  • Tests passed
  • Passes git diff origin/master **/*py | flake8 --diff
  • Fully documented

@sjoro sjoro marked this pull request as ready for review May 15, 2020 10:08
@coveralls
Copy link

coveralls commented May 15, 2020

Coverage Status

Coverage remained the same at 0.0% when pulling 0607bd9 on sjoro:vii into 24552f0 on pytroll:master.

@sjoro sjoro marked this pull request as draft May 15, 2020 10:29
@sjoro
Copy link
Contributor Author

sjoro commented May 15, 2020

Many lint errors come from the test_viiinterpolator.py

TEST_LON_2 = np.array(
    [[-12.        , -11.50003808, -11.        , -10.50011426,
       -9.        ,  -8.50026689,  -8.        ,  -7.50034342],
     [ -9.00824726,  -8.50989187,  -8.01100418,  -7.51272848,
       -6.01653996,  -5.51842688,  -5.01932226,  -4.52129225],
     [ -6.        ,  -5.50049716,  -5.        ,  -4.50057447,
       -3.        ,  -2.50073021,  -2.        ,  -1.50080874],
     [ -3.02492451,  -2.52706443,  -2.02774808,  -1.52997501,
       -0.03344942,   0.46414517,   0.96366893,   1.4611719 ],
     [  0.        ,   0.49903263,   1.        ,   1.49895241,
        3.        ,   3.49878988,   4.        ,   4.49870746],
     [  2.9578336 ,   3.45514812,   3.9548757 ,   4.4520932 ,
        5.94886832,   6.44588569,   6.94581415,   7.44272818]]
)

if white spaces are removed, readability is compromised.
what's the suggesed course of action here?

@sjoro sjoro marked this pull request as ready for review May 15, 2020 14:41
@mraspaud
Copy link
Member

Either add # noqa at the end of each offending line, or remove the spaces.

@sjoro
Copy link
Contributor Author

sjoro commented May 19, 2020

i removed the spaces.

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

Thanks for the nice work! This is just a first review, since I need more information on how the interpolation scheme goes to be able to review the algorithmic par of this.

Also, could you add a page in the documentation for this module ?

geotiepoints/viiinterpolator.py Show resolved Hide resolved
geotiepoints/viiinterpolator.py Show resolved Hide resolved
geotiepoints/viiinterpolator.py Show resolved Hide resolved
geotiepoints/viiinterpolator.py Outdated Show resolved Hide resolved
geotiepoints/viiinterpolator.py Outdated Show resolved Hide resolved
geotiepoints/viiinterpolator.py Outdated Show resolved Hide resolved
geotiepoints/viiinterpolator.py Outdated Show resolved Hide resolved
geotiepoints/viiinterpolator.py Outdated Show resolved Hide resolved
geotiepoints/viiinterpolator.py Outdated Show resolved Hide resolved
geotiepoints/viiinterpolator.py Outdated Show resolved Hide resolved
@sjoro sjoro requested a review from mraspaud June 4, 2020 09:23
@sjoro
Copy link
Contributor Author

sjoro commented Jun 4, 2020

Also, could you add a page in the documentation for this module ?

I have raised an issue on this. Link to the documentation should be added once publicly available.

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

Looks good, just one thing to fix!

geotiepoints/tests/test_viiinterpolator.py Outdated Show resolved Hide resolved
AUTHORS.md Outdated Show resolved Hide resolved
@mraspaud mraspaud merged commit 875124b into pytroll:master Jun 5, 2020
@sjoro sjoro deleted the vii branch June 5, 2020 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants