Conversation
| self._id = f"{self.name}_{id(self)}" | ||
| return self._id | ||
|
|
||
| def __hash__(self) -> int: |
There was a problem hiding this comment.
To check if I understood it right, the hash will now be generated based on the entire object, and not only on its id, correct?
There was a problem hiding this comment.
Yeah, I'm not sure why I put this in in the first place.
There was a problem hiding this comment.
Actually, it may be worth checking how targets are copied, if ever. I believe deepcopy changes the hash but not the id, so this could potentially lead to issues. I'll take a closer look at some point; not a big enough performance improvement to warrant potentially introducing an error without being sure of this.
There was a problem hiding this comment.
I hadn't considered that before, but I agree. This is a subtle change that can lead to unexpected consequences.
Mark2000
force-pushed
the
Mark2000-patch-1
branch
from
706725d to
17268ba
Compare
|
Closing, included in #301 |
2 participants
Description
This is slower than the python default hash function for the Target object.
Type of change
How should this pull request be reviewed?
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Passes Tests
pytest --cov bsk_rl --cov-report term-missing tests/unittestpytest --cov bsk_rl --cov-report term-missing tests/integrationcd docs; make htmlTest Configuration
Checklist:
Issue #XXX: Messageand have a useful message