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

"Unknown conversion specified" in test suite with pint 0.18 #31

Closed
phackstock opened this issue Nov 2, 2021 · 8 comments · Fixed by #35
Closed

"Unknown conversion specified" in test suite with pint 0.18 #31

phackstock opened this issue Nov 2, 2021 · 8 comments · Fixed by #35
Assignees

Comments

@phackstock
Copy link
Collaborator

A new version of pint (0.18) was released last week (2021.10.26), which caused the pyam tests connected to units to fail.
An easy and quick fix would be to pin the version of pint <= 0.17.
Additionally, it might be a good idea to set up nightly tests that run at least once a week so that we catch these dependency changes.

@khaeru
Copy link
Contributor

khaeru commented Nov 2, 2021

Can we add some information on how the tests fail, and what exactly was the offending change in Pint (e.g. a link to the changelog)?

Introducing pins in setup.cfg is not totally verboten, but shouldn't be done without first considering how difficult it would be to immediately follow changes upstream. If the answer is "very difficult," then we can consider it.

@khaeru
Copy link
Contributor

khaeru commented Nov 9, 2021

Can we add some information on how the tests fail, and what exactly was the offending change in Pint (e.g. a link to the changelog)?

Since this is blocking #34, just opened, I will do it.

@khaeru
Copy link
Contributor

khaeru commented Nov 9, 2021

The error messages (e.g. here) look like:

iam_units/__init__.py:136: in format_mass
    return format_unit(to_units_container(dict(units), registry=registry), spec)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

unit = <UnitsContainer({'kilogram CO2': 1, 'year': -1})>, spec = ':C'
registry = None, options = {}, fmt = None

    def format_unit(unit, spec, registry=None, **options):
        # registry may be None to allow formatting `UnitsContainer` objects
        # in that case, the spec may not be "Lx"
    
        if not unit:
            if spec.endswith("%"):
                return ""
            else:
                return "dimensionless"
    
        if not spec:
            spec = "D"
    
        fmt = _FORMATTERS.get(spec)
        if fmt is None:
>           raise ValueError(f"Unknown conversion specified: {spec}")
E           ValueError: Unknown conversion specified: :C

/opt/hostedtoolcache/Python/3.10.0/x64/lib/python3.10/site-packages/pint/formatting.py:408: ValueError

I also see in places:

E           ValueError: Unknown conversion specified: :~

@khaeru
Copy link
Contributor

khaeru commented Nov 9, 2021

The changelog for pint 0.18 is here. Nothing jumps out. But someone opened hgrecco/pint#1407 2 days after this issue, so it appears to be an unanticipated breakage and possibly headed towards resolution.

@khaeru
Copy link
Contributor

khaeru commented Nov 9, 2021

@phackstock if you also want to monitor for pint 0.18.1 and, once it's released, make another PR to revert #32, then we can approve and merge that PR. Otherwise, I'd prefer to wait 🙂

@danielhuppmann
Copy link
Member

I'm not sure I interpret the discussion in the pint-repo (hgrecco/pint#1407) that this was an unanticipated change to be resolved soon - sounds to me like they'll just reinsert and properly deprecate the breaking change.

So I suggest that we add the pin, and @phackstock puts it on his to-do list for next week to investigate how this can be properly fixed.

@phackstock
Copy link
Collaborator Author

I subscribed to the issue and set 'watch' for new releases so I should be notified if anything changes there.
But the way I read it, is that it is intended behavior that they just didn't document well. The strategy they were discussing is rolling it back for 18.1 but including it for good in 19.0. Still that would move the problem for us.

@khaeru khaeru self-assigned this Nov 11, 2021
@khaeru khaeru changed the title New version of pint crashes tests "Unknown conversion specified" in test suite with pint 0.18 Nov 11, 2021
@khaeru
Copy link
Contributor

khaeru commented Nov 11, 2021

If the change is permanent, then we should adjust to it. I went ahead and looked, and it turns out that's simple. See #35.

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

Successfully merging a pull request may close this issue.

3 participants