Skip to content

Conversation

@olivierbonte
Copy link
Contributor

Suggestions to improve type stability of some functions. .

@bgctw
Copy link
Member

bgctw commented Aug 1, 2025

Thanks Oliver for those suggestions. Can you, please, change some of the hard constructors to the suggested usage of convert?, And can you, please, resolve the conflict in unit_conversions.jl, before I merge it to main?

@olivierbonte
Copy link
Contributor Author

Thanks for the feedback, conflict is resolved and as as suggested changed use of FT(...) to convert(FT,...).

@bgctw
Copy link
Member

bgctw commented Aug 1, 2025

For constants I prefer the FT(...) over the convert, then the Julia compiler inserts the correctly typed value directly in the compiled code, e.g. FT(0.3). If you you are not bored by this picky issue, please, change those back to the more readable hard cast. Otherwise, I can merge it the way it is.

@bgctw bgctw merged commit c93d9b9 into EarthyScience:main Aug 4, 2025
4 checks passed
@olivierbonte
Copy link
Contributor Author

Sorry for late reply, was planning on changing back to FT(...) now, but I see the PR is already merged. Still made the commit olivierbonte@eabb4be. Made a new PR just in case you still want to adapt it to be this way, otherwise feel free to decline the new PR, found at #58

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.

2 participants