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

ENH: Add Coordinate transformations from pysat #6

Merged
merged 60 commits into from
Aug 17, 2020
Merged

ENH: Add Coordinate transformations from pysat #6

merged 60 commits into from
Aug 17, 2020

Conversation

jklenzing
Copy link
Member

@jklenzing jklenzing commented Jul 3, 2020 •
edited

Addresses #7

Adds coordinate routines related to JRO from pysat. Includes pysat history for contribution credit.

Also adds a changelog

jklenzing and others added 30 commits February 13, 2019 08:09
@jklenzing
utils restructure - #124
6f7f7df
@jklenzing
restructure
c6870ed
@jklenzing
longitude functions to coords
49a3894
@jklenzing
rename test files and tidy
2e74182
@jklenzing
test_utils_coords structure
8046231
@jklenzing
move local time to coords
9129d13
@jklenzing
unit tests for solar local time
c7da44e
@jklenzing
more coord tests + style
e292e42
@jklenzing
test_calc_solar_local_time
99e196f
@jklenzing
loosen tolerance on test_calc_solar_local_time
1ec067c
@jklenzing
updated targets
d27d4a7
@jklenzing
test
a6f27ea
@jklenzing
opposite test
62ddd17
@jklenzing
review round 1
76a68d8
@jklenzing
setitem fix
086a200
@jklenzing
test
0e33e06
@jklenzing
typo
dadd14a
@jklenzing
switch to mod
5bf8a8b
@jklenzing
restore travis tests
862af2f
@jklenzing
added unit test
44f068c
@jklenzing
add unit testing
c4bbba7
@jklenzing
fix for #171
9573c5f
@jklenzing
Merge branch 'develop' into documentation
173fa32
@jklenzing
Merge branch 'develop' into documentation
08b25d1
@jklenzing
Merge pull request #175 from rstoneback/documentation
583c843 
Documentation update
@jklenzing
tidy up imports
fad618a
@jklenzing
Merge pull request #192 from rstoneback/more_testing
9f3eb95 
More testing
@aburrell
Update coords.py
706241e 
Updated failures to be more informative, allowing more intellegent error catches.
@aburrell
Update coords.py
bf17f58 
- Added new unit possibilities for velocity
- Reduced the amount of time spent in initial for-loop
@aburrell
Update test_utils_coords.py
5bfded5 
- Added tests for new functionality
aburrell
aburrell reviewed Jul 13, 2020
View reviewed changes
Copy link
Member

@aburrell aburrell left a comment

Choose a reason for hiding this comment

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

Style comments for cleaning up and parametrizing unit tests.

pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
aburrell and others added 3 commits July 13, 2020 10:54
@aburrell
BUG: downstream changes
b12b90d 
Fixed downstream use of `coords` to use local coordinate routine.  Also fixed
import order.
@jklenzing
TST: modernize geod to geoc tests
40fce95
@jklenzing
Merge branch 'coords' of https://github.com/pysat/pysatMadrigal into …
85ab450 
…coords
@jklenzing
Copy link
Member Author

Pushed proposed structural changes for the geodetic to geocentric tests. If we are fully embracing parametrize, then the setup teardown class structure isn't needed.

@jklenzing jklenzing marked this pull request as draft July 23, 2020 16:02
@jklenzing
Copy link
Member Author

Marking as draft until group discussion on test styles for path forward

@jklenzing
Copy link
Member Author

Notes for moving forward:

  • Use hybrid approach for class / parametrize
  • always include setup and teardown, even if blank
  • test default options first, then kwargs in parametrize

@jklenzing jklenzing linked an issue Aug 15, 2020 that may be closed by this pull request
Geographic and Geodetic Coordinates #7
Closed
@jklenzing
Copy link
Member Author

failures due to changing pysat standards. Fixed in #15

@jklenzing
Copy link
Member Author

Rewrite based on 8/12 discussion ready for review

@jklenzing jklenzing marked this pull request as ready for review August 15, 2020 23:48
aburrell
aburrell reviewed Aug 17, 2020
View reviewed changes
Copy link
Member

@aburrell aburrell left a comment

Choose a reason for hiding this comment

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

Suggestions applied to unit tests can be used in other unit test classes in the same file, reducing the number of lines.

pysatMadrigal/instruments/jro_isr.py
import numpy as np

from pysatMadrigal.instruments.methods import madrigal as mad_meth
from pysat.instruments.methods import general as mm_gen
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from pysat.instruments.methods import general as mm_gen
from pysat.instruments.methods import general as ps_gen

Copy link
Member

Choose a reason for hiding this comment

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

With appropriate changes made below.

pysatMadrigal/tests/test_utils_coords.py Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
pysatMadrigal/tests/test_utils_coords.py Outdated Show resolved Hide resolved
jklenzing and others added 3 commits August 17, 2020 09:30
@jklenzing
Copy link
Member Author

Thanks for the suggestions! It's a lot cleaner now.

aburrell
aburrell approved these changes Aug 17, 2020
View reviewed changes
Copy link
Member

@aburrell aburrell left a comment

Choose a reason for hiding this comment

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

Thanks! It's a lot easier for me to read now.

@jklenzing jklenzing merged commit e250c86 into develop Aug 17, 2020
@jklenzing jklenzing deleted the coords branch August 17, 2020 14:50
@jklenzing jklenzing mentioned this pull request Aug 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aburrell aburrell aburrell approved these changes

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

Successfully merging this pull request may close these issues.

Geographic and Geodetic Coordinates
3 participants
@jklenzing @aburrell @rstoneback