Should UnitStore _is_unit_defined be private

[skeating]

Currently UnitStore function _is_unit_defined is private. I certainly need it for unit conversion and would imagine it would be a useful function.

Thoughts ??

[jonc125]

If you're using it from outside, then yes, makes sense to make it public!

[jonc125]

Though given we can pass Unit objects around now rather than always having to use names, it might be less important?

[skeating]

No, if I was passing a Unit object I still need to know if it is defined in the model.

[skeating]

Or I suppose I could just add it !

[jonc125]

That was kind of my point: we shouldn't necessarily need to add units just needed for conversions as named units to the model, as we can just hold the definition objects.

[skeating]

Hmm, maybe confused (or its Friday pm !).
In doing unit conversion I have add_input('variable_name', 'unit_string') and the underlying model must know what 'unit_string' refers to. My current thinking was that if add_input could not find 'unit_string' it refused conversion.

Passing a Unit object would mean it is less likely I add a unit that already exists with perhaps a new 'name'. Actually it would make more sense for a Unit object to be passed as that way the user MUST have fully defied the unit - not just assumed we know what they meant by 'mV'.

That way we only add a unit if we need to assign it to a variable.

Is that what you meant ?

[jonc125]

Yes, that is basically what I meant. It is what @MichaelClerx has been doing in #172.

[skeating]

Okay closing this as I will pass Unit objects in my conversion code.

If anyone else thinks the function should be public, please reopen.