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

Fix mutating the application registry #1403

Merged
merged 5 commits into from
Nov 23, 2021

Conversation

keewis
Copy link
Contributor

@keewis keewis commented Nov 1, 2021

When adding the ApplicationRegistry class I apparently forgot to forward __setattr__. This means that trying to mutate the application registry does not change the wrapped registry, but rather adds a new attribute to ApplicationRegistry:

from pint import application_registry as ureg

ureg.force_ndarray_like = True
assert ureg.force_ndarray_like is True
assert ureg._registry.force_ndarray_like is True  # AssertionError

Since this is included in v0.18, we might need a new release soon. What do you think, @hgrecco, @jules-ch

  • Executed pre-commit run --all-files with no errors
  • The change is fully covered by automated unit tests
  • Added an entry to the CHANGES file

@keewis keewis mentioned this pull request Nov 1, 2021
1 task
@keewis
Copy link
Contributor Author

keewis commented Nov 1, 2021

I'm not sure if that was a random failure on RTD, but pinning sphinx>4 seems to have worked

@jules-ch
Copy link
Collaborator

jules-ch commented Nov 1, 2021

I've explained the error in RTD build here #1399 (comment).
We need to get rid of those hard coded dependencies. We can pin sphinx version like you did in the meantime

Looks good to me.
We can do a 0.18.1 release

@keewis
Copy link
Contributor Author

keewis commented Nov 1, 2021

great. Should I add a note to CHANGES?

@keewis
Copy link
Contributor Author

keewis commented Nov 1, 2021

oh, and is it fine to keep the pin in this PR?

@jules-ch
Copy link
Collaborator

jules-ch commented Nov 1, 2021

Yes Add this fix in CHANGES & keep the pin I'll pin other docs dependencies in another PR.

@keewis
Copy link
Contributor Author

keewis commented Nov 1, 2021

add this fix in CHANGES

done

@jules-ch jules-ch merged commit 79a0db2 into hgrecco:master Nov 23, 2021
@jules-ch
Copy link
Collaborator

Thanks @keewis

@keewis keewis deleted the fix-application-registry branch November 24, 2021 12:23
dalito pushed a commit to dalito/pint that referenced this pull request Nov 29, 2021
* check that mutations are propagated

* define __setattr__ on the ApplicationRegistry class

* try pinning sphinx [skip ci]

* update CHANGES [skip ci]
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 this pull request may close these issues.

2 participants