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

Consider factoring our the pint cf units module into its own package #1788

Open
1 task done
ocefpaf opened this issue Jun 20, 2024 · 8 comments
Open
1 task done

Consider factoring our the pint cf units module into its own package #1788

ocefpaf opened this issue Jun 20, 2024 · 8 comments
Labels
API Interfacing and User Concerns information For development/intsructional purposes standards / conventions Suggestions on ways forward

Comments

@ocefpaf
Copy link

ocefpaf commented Jun 20, 2024

Generic Issue

I started writing a cf_units alternative, to avoid the udunits2 dependency, and found out that 99.99% of what I need is already here in xclim. Would the devs here consider factoring this part out into its own package? I know at least half a dozen other packages that would use it as a dependency.

IMO, it more packages use it, more improvements can be done to support all of udunits2 and repleace cf_units. What do you think?

PS: If that is a big ask would the devs here be OK with me vendoring this part in another codebase?

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Zeitsperre Zeitsperre added standards / conventions Suggestions on ways forward information For development/intsructional purposes API Interfacing and User Concerns labels Jun 25, 2024
@Zeitsperre
Copy link
Collaborator

Hi Filipe, thanks for raising this issue!

I know that we've talked about moving much of this code to other more generalized and upstream packages (cf-xarray, pint, etc.), but I'm not sure where we stand currently. If you were willing to help with maintenance, I can see the value in spinning off this code to another package altogether.

Maintenance of the boilerplate is easy as we have our own cookiecutter template that we version with cruft and we could start something new relatively quickly. My major concerns are ensuring that we can keep this package up-to-date with pint and xarray changes.

I'll let my colleagues weigh in on this here before we decide whether to move forward on this idea.

FYI @Ouranosinc/xclim-core

@ocefpaf
Copy link
Author

ocefpaf commented Jun 26, 2024

BTW,in case you are curious, this is my application that would use it: https://github.com/ioos/compliance-checker/pull/1094/files

For now I'm copying over parts of cf-xarray, xclim, and some code from them that comes from metpy. I already added few units but I do plan to parse the udunits2 xml file and add them all. Working on the COARDS dates first b/c that is the main blocker for us.

@aulemahal
Copy link
Collaborator

Hi ! I think we would be quite happy to have xclim's unit handling move somewhere else upstream. Are there specific parts you'd want to have accessible outside xclim ? We have a (heavy?) machinery around the flux<->rate<->amount conversion that uses CF names, is that of interest too ?

Sadly however, I don't think I have myself much time available to do so...

@Zeitsperre
Copy link
Collaborator

Hey @ocefpaf, we've had some time to talk this over, and we're relatively comfortable splitting these functions out into a new package. I guess what we'd want to know are the following:

  • Where do we see this being hosted? (xarray-contrib? Ouranosinc?)
  • Are we comfortable giving the core maintainers here (@aulemahal, @tlogan2000, myself, and a few others) and yourself admin access?
  • What should we call it?
  • Are we comfortable using our cookiecutter template?

Beyond that, there are a few functions that we rely on that are very closely tied to xarray and CF conventions, making us wonder how this package will differ from the aims of cf-xarray.

Conveniently, a change like this would really help with another goal we have in mind: https://github.com/Ouranosinc/xsdba

@ocefpaf
Copy link
Author

ocefpaf commented Jul 10, 2024

  • Where do we see this being hosted? (xarray-contrib? Ouranosinc?)

No strong preference. I'm fine with whatever you believe is better.

  • Are we comfortable giving the core maintainers here (@aulemahal, @tlogan2000, myself, and a few others) and yourself admin access?

+1

  • What should we call it?

I'm terrible with names... udunitspy is taken on PyPI (also gravitates around udunits), so is cf_units. Maybe clim_units?

  • Are we comfortable using our cookiecutter template?

Never used it but I can manage. Note that, while I'd love to get this done ASAP. I don't have the cycles for it right now. If my day-job contract is renewed I will probably tackle this.

@ocefpaf
Copy link
Author

ocefpaf commented Jul 10, 2024

We have a (heavy?) machinery around the flux<->rate<->amount conversion that uses CF names, is that of interest too ?

To be honest, I don't know but it is likely. At this point we need a cf_units alternative that does not have a C library, like udunits, under the hood. My primary goal is to add as many udunits units as possible to get our software going.

@aulemahal
Copy link
Collaborator

But are you looking for a array-agnostic solution i.e. that works for xarray as well as iris or pure numpy ? Because, I would rather be in favor of moving things to cf-xarray if possible.

@ocefpaf
Copy link
Author

ocefpaf commented Jul 10, 2024

Because, I would rather be in favor of moving things to cf-xarray if possible.

Ideally cf-xarray would use this package as well. I'm not sure how low level we can be but maybe this package would be just a set of new units (cf/udunits) registered in pint. The array functionality can be another package and depend on it. That would help keep the dependencies low for projects that would use this just to check units, or want to implement their own internal machinery.

With that said, as long as it is pure Python, most projects can handle xarray as a dependency, it is not a big price to pay. Having a single package that does both new units + xarray support would be acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Interfacing and User Concerns information For development/intsructional purposes standards / conventions Suggestions on ways forward
Projects
None yet
Development

No branches or pull requests

3 participants