NOAA-GFDL / xwmt

Python package for water mass transformation analysis that leverages xarray functionality
https://xwmt.readthedocs.io/
MIT License
7 stars 5 forks source link

Non-conformance in names #28

Open jkrasting opened 2 years ago

jkrasting commented 2 years ago

@jetesdal and @gmacgilchrist - there are a lot of variables, attributes, methods, etc. that could have more descriptive names. As a general rule, they should be longer than 3 characters and be all lower case.

We can exempt a small number if there is a good reason, (e.g. ds is commonly used throughout the community to denote an xarray.core.Dataset object, but most of these could be made more intuitive.

Can you suggest some alternatives?

Variables -- Must be longer than three characters and all lower case

Attributes -- Must be longer than three characters and all lower case

Functions -- Must be longer than three characters and all lower case

Class Methods -- Must be longer than three characters and all lower case

gmacgilchrist commented 2 years ago

I wonder if retaining some of the capitalization might be important, to be consistent with mathematical nomenclature? For example, Lambda (or L) is the vertically-intergrated version of lambda (l); that is, capitalization denotes the exensive quantitity. Capital J is also a common mathematical notation for tracer fluxes, while capital Q is common for surface mass fluxes. These latter two are more easily replaced.

Anyway, here are suggestions for some of these...

Variables -- Must be longer than three characters and all lower case

Attributes -- Must be longer than three characters and all lower case

Functions -- Must be longer than three characters and all lower case

Class Methods -- Must be longer than three characters and all lower case

hdrake commented 1 year ago

I wonder if retaining some of the capitalization might be important, to be consistent with mathematical nomenclature? For example, Lambda (or L) is the vertically-intergrated version of lambda (l); that is, capitalization denotes the exensive quantitity.

I agree with Graeme that it is more important for the capitalization convention for these terms to be consistent with the mathematical convention that the data formatting conventions, at least internally in the package.

I can work on implementing @gmacgilchrist's suggested renaming if others agree.

jkrasting commented 1 year ago

The Python programmer in me advises against this. The PEP-8 standard calls for lowercase variable names. The recommended way around this is to add descriptive suffixes, e.g. lambda_vint

hdrake commented 1 year ago

Thanks for the input, @jkrasting. I hadn't known the convention was so strict for variables/functions.

The recommended way around this is to add descriptive suffixes, e.g. lambda_vint

I am satisfied with just using suffixes like this.

gmacgilchrist commented 1 year ago

Staying close to conventional standards feels like the right move.

hdrake commented 1 year ago

As a general rule, they should be longer than 3 characters and be all lower case.

Where does this convention come from? I don't see it in the PEP8 style guide.

jkrasting commented 1 year ago

You’re right, @hdrake. PEP-8 does not specify a minimum length, but some Python linters enforce this convention for clarity. Pylint is good example of this.