fix: add extra config to make Unit model hashable #187
Conversation
|
@tstorek I have made the changed you suggested in this branch, can you add a test for caching under |
tstorek
changed the title
|
@djs0109 I added the test you requested. Including the example how lru_cache works ;) it raises perfomrance a lot with repeating function calls |
|
Thanks for providing this example 👍 I can see a speed boost of about 100-500 times |
CHANGELOG.md
Outdated
| @@ -1,3 +1,6 @@ | |||
| #### v0.2.5 | |||
| - fixed inconsistency of `entity_type` as required argument ([#188](https://github.com/RWTH-EBC/FiLiP/issues/188)) | |||
|
|
|||
There was a problem hiding this comment.
please update before merge or do you autogenerate from here on? Probably better
filip/semantics/semantics_models.py
Outdated
| @@ -1132,7 +1132,7 @@ def remove(self, v): | |||
| elif isinstance(v, SemanticIndividual): | |||
| self._set.remove(v.get_name()) | |||
| else: | |||
| raise KeyError | |||
| raise KeyError(f"v is neither of type SemanticIndividual nor SemanticClass but {type(v)}") | |||
There was a problem hiding this comment.
The error use different code patterns
| @@ -64,7 +64,7 @@ def test_unit_model_caching(self): | |||
|
|
|||
| self.assertEqual(unit.__hash__(), unit.__hash__()) | |||
|
|
|||
| @functools.lru_cache | |||
| @functools.lru_cache(maxsize=128) | |||
There was a problem hiding this comment.
isn't that the default anyway? I don't think that python 3.7 should fail because of this. https://docs.python.org/3.7/library/functools.html
There was a problem hiding this comment.
I also wonder about that, but it actually fails with the error message:
TypeError: Expected maxsize to be an integer or None
btw, the python examples all give the maxsize explicitly.
|
@djs0109 is this one fiex now? Probabably, we should double check by merging the main branch into it :) |
|
@tstorek it should have been fixed. But you are right, we should double check, because it is implemented before the pydantic migration. I will try to solve the conflict next week |
# Conflicts: # CHANGELOG.md # filip/models/ngsi_v2/units.py
close #186