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

Test_protocol_interface fails on my machine #221

Open
MichaelClerx opened this issue Oct 26, 2020 · 10 comments
Open

Test_protocol_interface fails on my machine #221

MichaelClerx opened this issue Oct 26, 2020 · 10 comments

Comments

@MichaelClerx
Copy link
Contributor

___________________________________________________________________________ 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
Copy link
Contributor Author

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

@MichaelClerx
Copy link
Contributor Author

@MichaelClerx
Copy link
Contributor Author

MichaelClerx commented Oct 26, 2020

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
Copy link
Contributor Author

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

@MauriceHendrix
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

MichaelClerx commented Oct 27, 2020

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
Copy link
Contributor

jonc125 commented Oct 27, 2020

I'd be happy to bump up!

@MauriceHendrix
Copy link
Contributor

bumping should be ok

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

3 participants