ModellingWebLab / weblab-fc

Functional Curation backend for the Modelling Web Lab
Other
1 stars 0 forks source link

Test_protocol_interface fails on my machine #221

Open MichaelClerx opened 4 years ago

MichaelClerx commented 4 years ago
___________________________________________________________________________ test_protocol_interface ____________________________________________________________________________
test/test_protocol_interface.py:82: in test_protocol_interface
    assert expected == actual
E   AssertionError: assert [{'kind': 'output', 'name': 'missing_units', 'units': ''},\n {'kind': 'output', 'name': 'model_interface_units', 'units': '0.001 second'},\n {'kind': 'output', 'name': 'pp_defined_units', 'units': '0.001 second'},\n {'kind': 'output', 'name': 'sim_defined_units', 'units': '1 second'},\n {'kind': 'output', 'name': 'unknown_units', 'units': ''}] == [{'kind': 'output', 'name': 'missing_units', 'units': ''},\n {'kind': 'output', 'name': 'model_interface_units', 'units': '0.001 second'},\n {'kind': 'output', 'name': 'pp_defined_units', 'units': '0.001 second'},\n {'kind': 'output', 'name': 'sim_defined_units', 'units': '1.0 second'},\n {'kind': 'output', 'name': 'unknown_units', 'units': ''}]
E     At index 3 diff: {'kind': 'output', 'name': 'sim_defined_units', 'units': '1 second'} != {'kind': 'output', 'name': 'sim_defined_units', 'units': '1.0 second'}
E     Full diff:
E       [
E        {'kind': 'output', 'name': 'missing_units', 'units': ''},
E        {'kind': 'output', 'name': 'model_interface_units', 'units': '0.001 second'},
E        {'kind': 'output', 'name': 'pp_defined_units', 'units': '0.001 second'},
E     -  {'kind': 'output', 'name': 'sim_defined_units', 'units': '1 second'},
E     +  {'kind': 'output', 'name': 'sim_defined_units', 'units': '1.0 second'},
E     ?                                                             ++
E        {'kind': 'output', 'name': 'unknown_units', 'units': ''},
E       ]
MichaelClerx commented 4 years ago

The unit string is added by the cellmlmanip unit store, here: https://github.com/ModellingWebLab/weblab-fc/blob/master/fc/protocol.py#L669

MichaelClerx commented 4 years ago

The unit string is made here https://github.com/ModellingWebLab/cellmlmanip/blob/master/cellmlmanip/units.py#L225-L226

MichaelClerx commented 4 years ago

This is due to changes in pint since version 0.10, which I've got installed, possibly these ones in 0.11:

https://github.com/hgrecco/pint/blob/master/CHANGES#L118-L119

Updating pint to 0.16 fixes the issue for me

MichaelClerx commented 4 years ago

@jonc125 @MauriceHendrix what do you think, should we raise the required version of PINT in cellmlmanip to the latest one?

MauriceHendrix commented 4 years ago

@MichaelClerx just ran a test with pint set to 0.16 in a PR and that passes all tests for all the tested python versions so it should be ok from my end.

MichaelClerx commented 4 years ago

Thanks. Do you think this issue, where the output of str(self._registry.get_base_units(unit)[0]) has changed from 1.0 to 1, is big enough for us to say "users have to use at least version 0,16" ?

MauriceHendrix commented 4 years ago

it doesn't seem to cause any issues for me. If it does for you in the weblab you might consider adjusting the printer to always just print 1 something along the lines of:

def _print_float(self, expr):
    """ Handles ``float``s. """
    if(float(<number>).is_integer()):
        return self._print_int(int(expr))
    else:
.....
MichaelClerx commented 4 years ago

It's not in the printer, the issue is that I call self._registry.get_base_units(unit)[0], which gives me a pint unit, and that unit has a __str__ method that involves printing the float or int, and in older versions it was always a float but now something it's an int. The result is that str(unit) gives a slightly different output now. Both outputs are correct, but it makes writing tests that use this output slightly harder

So my question is whether we should bump the required version of pint up to the latest version

jonc125 commented 4 years ago

I'd be happy to bump up!

MauriceHendrix commented 4 years ago

bumping should be ok