Conversation
| """Use milliseconds and provide a reason for the pause.""" | ||
| self.unit = f"millisecond{'s' if self.number != 1 else ''}" | ||
| self.time = self.time / 1000.0 | ||
| self.time = self.number / 1000.0 |
There was a problem hiding this comment.
Hmmm, it’s not wrong nor does it seem needed since it’s an alias var
There was a problem hiding this comment.
This one was interesting, the AI said that if someone accidentally called milliseconds_because() two times, it would divide the original number by 1,000 again, making 1/1,000,000. If we use self.number instead, this method becomes idempotent!
| from screenpy.narration import Narrator, StdOutAdapter | ||
| from screenpy.speech_tools import represent_prop | ||
| from .narration import Narrator, StdOutAdapter | ||
| from .speech_tools import represent_prop |
There was a problem hiding this comment.
Not against this, but is it better?
There was a problem hiding this comment.
Before, screenpy.pacing was the only file that used absolute imports; all the others use relative imports.
| import sys | ||
| from pathlib import Path | ||
| from typing import TYPE_CHECKING | ||
| import tomllib |
There was a problem hiding this comment.
Does this work in all versions of python?
There was a problem hiding this comment.
Hm... i may have been overzealous on this one. I took its recommendation and made the edits myself. I think i was only supposed to take out the try/except bit, because that handling was only necessary when 3.11 was in alpha.
2 participants
This PR has a bunch of edits suggested by the AI after i asked it "look at the whole repo and make suggestions for improvements." It mostly found typos and stuff, i was hoping for something more substantial but it is right—these should be fixed, too!