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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update tests to astropy 3.2 #700

Merged
merged 1 commit into from Jun 13, 2019
Merged

Update tests to astropy 3.2 #700

merged 1 commit into from Jun 13, 2019

Conversation

@jorgepiloto
Copy link
Member

jorgepiloto commented Jun 12, 2019

Just a small update on some tests due to astropy version 3.2 馃殌

Solving #665

@jorgepiloto jorgepiloto force-pushed the jorgepiloto:tests branch 2 times, most recently from 28d4693 to cac5b33 Jun 12, 2019
@jorgepiloto

This comment has been minimized.

Copy link
Member Author

jorgepiloto commented Jun 12, 2019

This push should be one. Sorry for the delay solving this, not sure why the tests were running properly on my local machine 馃槙

@jorgepiloto jorgepiloto force-pushed the jorgepiloto:tests branch from cac5b33 to 5ba3b03 Jun 12, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jun 12, 2019

Codecov Report

Merging #700 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #700   +/-   ##
=======================================
  Coverage   87.21%   87.21%           
=======================================
  Files          53       53           
  Lines        2676     2676           
  Branches      236      236           
=======================================
  Hits         2334     2334           
  Misses        267      267           
  Partials       75       75

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update f66c46c...996c4f0. Read the comment docs.

@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jun 12, 2019

Now the tests passed, but the question is: where in this code did the change happen?

def gcrs_to_geosolarecliptic(gcrs_coo, to_frame):
if not to_frame.obstime.isscalar:
raise ValueError(
"To perform this transformation the obstime Attribute must be a scalar."
)
_earth_orbit_perpen_point_gcrs = UnitSphericalRepresentation(
lon=0 * u.deg, lat=(90 * u.deg - _obliquity_rotation_value(to_frame.obstime))
)
_earth_detilt_matrix = _make_rotation_matrix_from_reprs(
_earth_orbit_perpen_point_gcrs, CartesianRepresentation(0, 0, 1)
)
sun_pos_gcrs = get_body("sun", to_frame.obstime).cartesian
earth_pos_gcrs = get_body("earth", to_frame.obstime).cartesian
sun_earth = sun_pos_gcrs - earth_pos_gcrs
sun_earth_detilt = sun_earth.transform(_earth_detilt_matrix)
# Earth-Sun Line in Geocentric Solar Ecliptic Frame
x_axis = CartesianRepresentation(1, 0, 0)
rot_matrix = _make_rotation_matrix_from_reprs(sun_earth_detilt, x_axis)
return matrix_product(rot_matrix, _earth_detilt_matrix)

@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jun 12, 2019

Perhaps @shreyasbapat has more information?

@jorgepiloto

This comment has been minimized.

Copy link
Member Author

jorgepiloto commented Jun 12, 2019

This may have the key:

What's new in Astropy 3.2?

@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jun 13, 2019

Spot the differences 馃槈

Screenshot from 2019-06-13 14-56-14

src/poliastro/tests/test_frames.py Outdated Show resolved Hide resolved
@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jun 13, 2019

@jorgepiloto jorgepiloto force-pushed the jorgepiloto:tests branch from 5ba3b03 to 996c4f0 Jun 13, 2019
@jorgepiloto

This comment has been minimized.

Copy link
Member Author

jorgepiloto commented Jun 13, 2019

Since the expected values were in utc scale, we finally decided to rewrite them in the tt one, according to J2000.

@Juanlu001 Juanlu001 merged commit 425112a into poliastro:master Jun 13, 2019
10 checks passed
10 checks passed
ci/circleci: coverage Your tests passed on CircleCI!
Details
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: quality Your tests passed on CircleCI!
Details
ci/circleci: test_py35 Your tests passed on CircleCI!
Details
ci/circleci: test_py36 Your tests passed on CircleCI!
Details
ci/circleci: test_py37 Your tests passed on CircleCI!
Details
codeclimate All good!
Details
codecov/patch Coverage not affected when comparing f66c46c...996c4f0
Details
codecov/project 87.21% remains the same compared to f66c46c
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can鈥檛 perform that action at this time.