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

Unintended conversion #12

Closed
znicholls opened this issue Dec 10, 2020 · 2 comments · Fixed by #18
Closed

Unintended conversion #12

znicholls opened this issue Dec 10, 2020 · 2 comments · Fixed by #18
Labels
bug Something isn't working

Comments

@znicholls
Copy link
Contributor

Describe the bug

At the moment it's possible to convert from NH3 to CO2 when in a context, this shouldn't happen.

Failing Test

def test_nh3_doesnt_convert():
    with pytest.raises(DimensionalityError):
        with unit_registry.context("AR5GWP100"):
            (1 * unit_registry("Mt NH3 / yr")).to("Mt CO2 / yr")

Expected behavior

Raise an error instead.

Screenshots

If applicable, add screenshots to help explain your problem.

System (please complete the following information):

  • OS: [e.g. Windows, Linux, macOS]
  • Python version [e.g. Python 3.5] and output of conda list --export and pip freeze as applicable

Additional context

Add any other context about the problem here.

@znicholls znicholls added the bug Something isn't working label Dec 10, 2020
@mikapfl
Copy link
Member

mikapfl commented Jan 5, 2021

This can probably only be solved by treating ammonia / NH3 like NOx and changing the definition from:

"NH3": ["14/17 * N", "ammonia"],

to

"NH3": "NH3",
"ammonia": ["NH3"],

and maybe adding an NH3_conversions context analogous to the NOx_conversions one.

Do you think this would be a useful solution? If you want to, I could write a pull request.

@znicholls
Copy link
Contributor Author

Do you think this would be a useful solution?

Yep perfect! If you have a moment, a PR would be much appreciated

@mikapfl mikapfl mentioned this issue Jan 6, 2021
4 tasks
znicholls added a commit that referenced this issue Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants