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

is_vertical? #15

Closed
ocefpaf opened this issue Mar 17, 2015 · 9 comments
Closed

is_vertical? #15

ocefpaf opened this issue Mar 17, 2015 · 9 comments
Milestone

Comments

@ocefpaf
Copy link
Member

ocefpaf commented Mar 17, 2015

is_vertical returns True for distances. That is the correct behavior, but the name is not all that intuitive.

@pelson
Copy link
Member

pelson commented Mar 18, 2015

It might be safest to get a tagged release of cf_units from which Iris can depend before we change the functionality too much. That way the work to get iris using cf_units rather than iris.units should be simpler.

What do you think?

@ajdawson
Copy link
Member

True, but on the other hand if the suggestion is a change to the public API of cf_units (a name change) then perhaps it should go in before a tagged release.

@ocefpaf
Copy link
Member Author

ocefpaf commented Mar 18, 2015

I will avoid changes to the Public API until we can tag a release, and I will tag a release as soon as I finish #14, #12, #3, #2.

Sounds like a plan?

@pelson
Copy link
Member

pelson commented Mar 18, 2015

Sounds like a plan?

👍 from me. I can see @ajdawson's point, so perhaps we should tag 0.1 in the Iris "unchanged" form, and then look to iterate quickly and if necessary move on to v1.x within a release or two.

@pelson
Copy link
Member

pelson commented Jan 6, 2016

Ping - is this still something you want to investigate @ocefpaf?

@ocefpaf
Copy link
Member Author

ocefpaf commented Jan 6, 2016

I would not rename it and Ido not believe we can return True for vertical units based only on the units.

Like: cf_units.is_vertical('millibar') might be always True, but
cf_units.is_vertical('km') can be vertical or horizontal.

We will use this method to confirm that the units from a known vertical coordinate has the correct length or pressure units.

Alternatives would be the horrible names: is_length_or_pressure(), is_valid_vertical_units(), or is_valid_vertical().

I guess that we could only add a note in the docs then!?

@rhattersley
Copy link
Member

Alternatives would be the horrible names: is_length_or_pressure(), is_valid_vertical_units(), or is_valid_vertical().

Or could_be_vertical(), might_be_vertical(), is_possibly_vertical(), ...

@ocefpaf
Copy link
Member Author

ocefpaf commented Jan 6, 2016

I knew someone could com up with worse names 😉

I read the docs and they are clear enough:

Determine whether the unit is a related SI Unit of pressure or distance.

My bad for opening this in the first place. Let's just close it.

@ocefpaf ocefpaf closed this as completed Jan 6, 2016
@rhattersley
Copy link
Member

I knew someone could com up with worse names 😉

😛

@pelson pelson modified the milestone: v1.1.0 Jan 6, 2016
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

No branches or pull requests

4 participants