TASK: Add Mood.express method (#66)#74
TASK: Add Mood.express method (#66)#74P-r-e-m-i-u-m wants to merge 4 commits intoTheGittyPerson:mainfrom
TASK: Add Mood.express method (#66)#74Conversation
Signed-off-by: 🄂ʏᴇᴅ 🄰ʙᴅᴜʟ 🄰ᴍᴀ🄝 ✧ <amanbaba9404522@gmail.com>
Signed-off-by: 🄂ʏᴇᴅ 🄰ʙᴅᴜʟ 🄰ᴍᴀ🄝 ✧ <amanbaba9404522@gmail.com>
Signed-off-by: 🄂ʏᴇᴅ 🄰ʙᴅᴜʟ 🄰ᴍᴀ🄝 ✧ <amanbaba9404522@gmail.com>
Signed-off-by: 🄂ʏᴇᴅ 🄰ʙᴅᴜʟ 🄰ᴍᴀ🄝 ✧ <amanbaba9404522@gmail.com>
| return "😐" | ||
|
|
||
| # Map 0.0–1.0 intensity to 1–10 scale | ||
| level = max(1, round(self.intensity * 10)) |
There was a problem hiding this comment.
round() applies banker's rounding, which means some intensity bands become uneven and shifts thresholds (e.g., 0.85 maps down to 8, 0.95 maps to 10). If the intent is uniform bins, consider a different rounding strategy.
| return "😐" | ||
|
|
||
| # Map 0.0–1.0 intensity to 1–10 scale | ||
| level = max(1, round(self.intensity * 10)) |
There was a problem hiding this comment.
Consider adding a check before this that raises an error to ensure intensity is within range.
You've correctly applied checks for intensity-setting methods above in your prior pull request, but intensity can still be accidentally set incorrectly with no filter just by doing person.mood.intensity = 69, for example.
Also, since you're the author of this entire module, could you please ensure this is the same for all other methods that use intensity?
| def __repr__(self) -> str: | ||
| """Return a detailed representation of the mood.""" | ||
| return f"Mood(name={self.name!r}, intensity={self.intensity})" | ||
|
|
There was a problem hiding this comment.
This is unrelated, but in the describe method above, I noticed it only checks self.name, so a non‑neutral mood with intensity == 0.0 still renders as “slightly ” instead of neutral. I assume your intention is to have non-neutral zero-intensity moods to be converted to and treated as neutral?
| Returns: | ||
| A single emoji string representing the mood. | ||
| """ | ||
| if self.name == "neutral": |
There was a problem hiding this comment.
My mistake, the above comment also applies here; if intensity == 0.0 for any mood, the neutral face should be returned.
I'd suggest adding a helper method to check whether name == 'neutral' AND name == 0.0 so that you can reuse it easily in other places. Maybe raise a value error within the method? unless you prefer to just have the method return True or False and have the error message raised where the method is used instead. (then please also apply the method in other places where necessary)
TheGittyPerson
added
enhancement
|
"Addressed review comments — added is_neutral(), fixed describe(), fixed express() rounding. Ready for re-review @TheGittyPerson |
2 participants
Closes #66
Added
express()method to theMoodclass that returns an emoji representing the current mood and intensity.Changes
theperson/mood.py: Addedexpress()with an_EMOJI_MAPcovering all 11 non-neutral moods