-
Notifications
You must be signed in to change notification settings - Fork 39
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
Fix longitude latitude confusion in extract_trajectory preprocessor #1174
Conversation
A minor edit to bextract_trajectory, where the latitude and longitude interface was backwards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch! isn't there a test as well that will need to be changed? or you used identical lat and lon points in the test 😁
The test is:
So it shouldn't fail as it's only effectively comparing the shape of the outputs. Whether that is a good enough unit test should like be another issue/PR! |
OK, it's a decent test, given that the numerical data parameters are tested for (cows vs bulls or bulls vs cows is a different matter, the test is happy as long as they're of the same shape then). I'm merging this if you don't need to get more stuff done to it 👍 |
Thanks V. Randomly, while I uncovered this bug in my local version of the code, I did all the editing and merging for this PR in github without ever having a local copy of this branch. It is a great method to quickly correct bugs. |
yes but be careful since some tests don't run on the pure github edited branch and only get run when merged onto main so it's good to do the bugfixes locally so you can run all the tests and see if there are issues before merging into main |
@ledm @valeriupredoi I'm just reviewing our changelog for v2.3 and I'm a bit disappointed by what you came up with as a title for this change. From our contribution guidelines:
Could you please put in a bit more effort next time? 🙏 |
Sorry, that was the title github generated. I'll change it now, post hoc. |
A minor edit to extract_trajectory, where the latitude and longitude interface was backwards.
Description
Partially addresses #1137.
Link to documentation:
Before you get started
Checklist
It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.
To help with the number pull requests: