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

Add some types #133

Open
enomado opened this issue Sep 14, 2022 · 2 comments
Open

Add some types #133

enomado opened this issue Sep 14, 2022 · 2 comments
Assignees
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@enomado
Copy link

enomado commented Sep 14, 2022

Hi. I share your passion to wellbore domain. Currently I'm creating interactive tools for showing trajectory, so probably there are some area for collaborations. I write my stuff it in Rust language. Previously I was using typescript, but stick on compiled language.

I saw you are using some sort of Units-Of-Measure like

unit_weight=(68 * ureg('lbs / ft').to('kg / meters')).m,

Its absolutely must-use feature IMO.

I recommend use typing more. For example
def maximum_curvature(survey, dls_noise=1.0):
What is survey? You can say its obvious its Survey class. Ok, can I click on it to go to Survey class definition? Can I type . and see its methods?

Annotate function signatures, remove *args, **kwargs so code will be much readable and ide-friendly.

Also I recomend NewType pattern for units. Lets say we have angle-param. Is it degrees or radians?

We can do some calc(Rad(1.0), Rad(0.1)) or something like AnglePair{ azimuth: Rad, zenith: Rad}

Or lets say we have NVEVec, and NVEVec.get_angles, or AnglePair.get_vector

Cheers.

@jonnymaserati
Copy link
Owner

Thanks for the feedback @enomado!

Pull requests are welcome if you'd like to propose any changes. It's increasingly difficult for me to make time to maintain the code and I prefer to focus my efforts on implementing new functionality.

On units, I did start with a branch to implement pint, but I could see that this had it downsides, notably making the code harder to read and some implications on speed. So my plan now is to implement a single unit system in the base modules and either add a wrapper for units or leave the conversion of units to the end user. The main reason for this is that the code is being used as a dependency in some projects where speed is major benefit.

Agree on the typing... when I started with this code, typing hadn't been implemented yet (python 3.6). Any new code will have typing and I'll slowly start adding it to existing code. Having said that, there are Google style docstrings for the main functions that generally hint at the typing and one day I'll get some proper documentation added.

@jonnymaserati jonnymaserati self-assigned this Sep 18, 2022
@jonnymaserati jonnymaserati added documentation Improvements or additions to documentation good first issue Good for newcomers labels Sep 18, 2022
@enomado
Copy link
Author

enomado commented Sep 20, 2022

Btw, Any thought on switch to Rust?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants