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 return_back_azimuth kwarg to fwd, inv, fwd_intermediate, inv_intermediate #1165

Merged
merged 5 commits into from
Dec 19, 2022
Merged

ENH: Add return_back_azimuth kwarg to fwd, inv, fwd_intermediate, inv_intermediate #1165

merged 5 commits into from
Dec 19, 2022

Conversation

idanmiara
Copy link
Contributor

@idanmiara idanmiara commented Oct 24, 2022 •
edited by snowman2
Loading

#1163

@snowman2 snowman2 added this to the 3.5.0 milestone Oct 24, 2022
@codecov
Copy link

codecov bot commented Oct 24, 2022 •
edited
Loading

Codecov Report

Merging #1165 (f944439) into main (e345729) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1165      +/-   ##
==========================================
+ Coverage   96.25%   96.28%   +0.02%     
==========================================
  Files          20       20              
  Lines        1791     1803      +12     
==========================================
+ Hits         1724     1736      +12     
  Misses         67       67
Impacted Files Coverage Δ
pyproj/geod.py 97.57% <100.00%> (+0.19%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@snowman2
Copy link
Member

Looks good, thanks 👍

Mind adding .. versionadded:: 3.5.0 return_back_azimuth to the methods that have this added?

@idanmiara idanmiara force-pushed the return_back_azimuth branch 6 times, most recently from 479811d to c7d91e3 Compare October 25, 2022 14:03
snowman2
snowman2 reviewed Oct 25, 2022
View reviewed changes
pyproj/utils.py Outdated Show resolved Hide resolved
@idanmiara
Copy link
Contributor Author

@snowman2 I think this is ready.
I could reorganize the commits if you approve this for merging.

snowman2
snowman2 approved these changes Oct 25, 2022
View reviewed changes
Copy link
Member

@snowman2 snowman2 left a comment

Choose a reason for hiding this comment

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

Thanks @idanmiara 👍

@snowman2
Copy link
Member

I am not going to merge it quite yet. 3.4.1 has not been released yet and this is targeted for 3.5. 3.4.1 may be replaced by 3.5, but not sure yet.

@idanmiara idanmiara force-pushed the return_back_azimuth branch 2 times, most recently from 5f9f8c3 to 39359cb Compare December 16, 2022 16:23
@snowman2
Copy link
Member

Thanks @idanmiara for the changes. We're getting closer. I updated the initial comment on the issue with a checklist of testing & documentation updates that would be good to add. Let me know if you have time to add those.

@idanmiara idanmiara force-pushed the return_back_azimuth branch 11 times, most recently from 7962a70 to a9b1b41 Compare December 17, 2022 07:41
@idanmiara
Copy link
Contributor Author

Thanks @idanmiara for the changes. We're getting closer. I updated the initial comment on the issue with a checklist of testing & documentation updates that would be good to add. Let me know if you have time to add those.

All good now?

snowman2
snowman2 reviewed Dec 17, 2022
View reviewed changes
pyproj/geod.py Outdated Show resolved Hide resolved
@snowman2 snowman2 mentioned this pull request Dec 18, 2022
Merged
@snowman2
Copy link
Member

This looks great - thanks @idanmiara 👍. I think this is ready to go. Would you like to clean up the commit history or would you mind if I squashed on merge?

@idanmiara
geod.py - Add reverse_azimuth function + add relevant tests
4364394
@idanmiara
add return_back_azimuth: bool = True to fwd()
98380a5
@idanmiara
add return_back_azimuth: bool = True to inv()
b0b0d4f
@idanmiara
Add return_back_azimuth: bool to _inv_or_fwd_intermediate
3f1de5b
@idanmiara
geod.py - Add return_back_azimuth: Optional[bool] = None to `inv_in…
f944439 
…termediate` and `fwd_intermediate` (`.. versionadded:: 3.5.0 return_back_azimuth`) to allow compatibility between the azimuth output of `fwd`/`fwd_intermediate` and to emit warning for the braking change; update `npts` to override the new default.

test_geod.py - update tests for the new default and add some new tests for new functionality
history.rst - document this change
@idanmiara
Copy link
Contributor Author

This looks great - thanks @idanmiara 👍. I think this is ready to go. Would you like to clean up the commit history or would you mind if I squashed on merge?

Thanks for your help!
I've cleaned the commit history.

@snowman2 snowman2 changed the title add return_back_azimuth: bool = False to allow compatibility between the azimuth output of fwd and fwd_intermediate ENH: Add return_back_azimuth kwarg to fwd, inv, fwd_intermediate, inv_intermediate Dec 19, 2022
@snowman2 snowman2 merged commit 1e309c2 into pyproj4:main Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snowman2 snowman2 snowman2 approved these changes

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

Successfully merging this pull request may close these issues.

Lack of consistency between fwd and fwd_intermediate azimuth output
2 participants
@idanmiara @snowman2