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

Spherical linestring curve #19

Merged
merged 9 commits into from
May 22, 2024
Merged

Spherical linestring curve #19

merged 9 commits into from
May 22, 2024

Conversation

louisgreiner
Copy link
Contributor

@louisgreiner louisgreiner commented Apr 24, 2024

Closes #12

  • implement SphericalLineStringCurve methods + tests
  • refacto LineStringCurve in PlanarLineStringCurve methods + tests + occurrences

@louisgreiner louisgreiner force-pushed the spherical_linestring_curve branch 3 times, most recently from 5974e37 to ea7dbbb Compare April 24, 2024 16:46
@louisgreiner
Copy link
Contributor Author

self.max_extend is public but has a getter, whats the point ?

  • -> either remove the getter
  • -> either change its range to private and add a setter

@louisgreiner
Copy link
Contributor Author

Should I remove some examples to enhance readability ?
such as example about brest -> nancy, or new_york -> paris

@louisgreiner
Copy link
Contributor Author

Should I round the coordinates in the tests section ? To enhance readability ?

@louisgreiner louisgreiner force-pushed the spherical_linestring_curve branch 2 times, most recently from 3163ea1 to 0466621 Compare April 24, 2024 16:58
Copy link
Contributor

@Tristramg Tristramg left a comment

Choose a reason for hiding this comment

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

The commits seem a bit a bit overlapping in their functionalities.
I don’t mind if we squash them, as untangling them would be way too complicated compared to the benefits

@louisgreiner louisgreiner force-pushed the spherical_linestring_curve branch 2 times, most recently from cd2bc18 to 4bca22b Compare May 3, 2024 12:48
@louisgreiner louisgreiner force-pushed the spherical_linestring_curve branch 2 times, most recently from d694be8 to c483623 Compare May 16, 2024 09:26
@louisgreiner louisgreiner force-pushed the spherical_linestring_curve branch from c483623 to 2d083b2 Compare May 16, 2024 09:37
@Tristramg Tristramg force-pushed the spherical_linestring_curve branch from 2d083b2 to ed7e000 Compare May 21, 2024 21:44
@Tristramg Tristramg merged commit be42f46 into main May 22, 2024
6 checks passed
@Tristramg Tristramg deleted the spherical_linestring_curve branch June 20, 2024 08:17
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 this pull request may close these issues.

Implement Curve for spherical linecurves
2 participants