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

Enforce sort order for winds #51

Closed
dbookstaber opened this issue Mar 11, 2024 · 5 comments · Fixed by #88
Closed

Enforce sort order for winds #51

dbookstaber opened this issue Mar 11, 2024 · 5 comments · Fixed by #88

Comments

@dbookstaber
Copy link
Contributor

In the case of multiple winds: TrajectoryCalc assumes that Shot.winds[] is sorted by Wind.until_distance (ascending). However right now we do not enforce that or even check whether that assumption is valid.

What is the best way to handle this?

@o-murphy
Copy link
Owner

Is this done?

@dbookstaber
Copy link
Contributor Author

No

@julius425
Copy link

Sorting can be done in Shot's post_init.
Hey guys, im new to open source. Im not a math guy, but i like guns. Can i try to commit to your project?

@dbookstaber
Copy link
Contributor Author

That would be great! The process to contribute:

  1. Fork the repository
  2. Make changes
  3. Ensure all unit tests pass
  4. Commit your changes to your fork
  5. Enter a "pull request" from your fork

Then the owner (@o-murphy) can review and accept your changes.

@o-murphy
Copy link
Owner

o-murphy commented May 2, 2024

Sorting can be done in Shot's post_init. Hey guys, im new to open source. Im not a math guy, but i like guns. Can i try to commit to your project?

it shouldn't be difficult, I think it can be done in one line, but I would do it with some setter method, cause Shot dataclass ain't frozen and we have to sort winds each times it changes, not just on postinit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants