#19: Fix typing for aliases. #20
Conversation
| @@ -13,6 +13,8 @@ | |||
| from .protocols import Forgettable, Performable | |||
| from .speech_tools import get_additive_description | |||
|
|
|||
| # pylint: disable=too-many-public-methods | |||
There was a problem hiding this comment.
I'm not sure why pylint was OK with all the aliases when they were created using = but complains now. This sure shut it up.
There was a problem hiding this comment.
The aliases within _TimeframeBuilder should get converted also.
millisecond = milliseconds
There was a problem hiding this comment.
I didn't convert them because they aren't surfaced for documentation (currently) and also do not have the problems that the classmethods did.
... Unless they do?
There was a problem hiding this comment.
Oh, you're right. They are just aliases. AFAICT they don't have the same problem as the classmethods.
| def with_ordered_cleanup_tasks(self, *tasks: Performable) -> "Actor": | ||
| """Alias for :meth:`~screenpy.actor.Actor.has_ordered_cleanup_tasks`.""" | ||
| return self.has_ordered_cleanup_tasks(*tasks) |
There was a problem hiding this comment.
I propose we simply drop the singular variants of these methods because it's purely an English detail.
That would render a total of 6 aliases comprising both types of cleanup tasks.
(or even better, 4 total if we just drop the ordered aliases and rely on independent cleanup tasks to already be distinguishing themselves from the implied ordered ones... but that's a separate discussion.)
If we do want to explicitly alias singular and plural forms as you have here then we should also include all the options by adding with_cleanup_tasks, has_ordered_cleanup_task, with_ordered_cleanup_task, has_independent_cleanup_task, and with_independent_cleanup_task.
This would bring us to a total of 11 12 methods spanning both types of cleanup task!
Personally I'd rather just accept a format in which has_{type}_cleanup_tasks() may contain one or more tasks, but you know my position is that English readability doesn't have to mean native speaker english readability in terms of precise tense agreement
the_actor.has_ordered_cleanup_tasks(ClickButton())
the_actor.has_independent_cleanup_tasks(
ClearTextField(),
LogOut(),
)There was a problem hiding this comment.
TL:DR -
A) (my preference) Dropping the singulars would leave us with:
has_cleanup_tasks()
has_ordered_cleanup_tasks()
with_cleanup_tasks()
with_ordered_cleanup_tasks()
has_independent_cleanup_tasks()
with_independent_cleanup_tasks()B) (something to consider) Also dropping the ordered variants and relying on the existing distinction already evident by the independent methods would leave us with
has_cleanup_tasks()
with_cleanup_tasks()
has_independent_cleanup_tasks()
with_independent_cleanup_tasks()C) (english stickler) And going the other direction with all singular and plural options would leave us with
has_cleanup_task()
has_cleanup_tasks()
with_cleanup_task()
with_cleanup_tasks()
has_ordered_cleanup_task()
has_ordered_cleanup_tasks()
with_ordered_cleanup_task()
with_ordered_cleanup_tasks()
has_independent_cleanup_task()
has_independent_cleanup_tasks()
with_independent_cleanup_task()
with_independent_cleanup_tasks()There was a problem hiding this comment.
I suppose we could go a different way with option (B) by dropping the less explicit form of the ordered variants and just stick with only the plurals of each e.g.
has_ordered_cleanup_tasks()
with_ordered_cleanup_tasks()
has_independent_cleanup_tasks()
with_independent_cleanup_tasks()There was a problem hiding this comment.
Haha, this is a great point. I think i agree with the streamline.
| def exit_stage_left(self) -> None: | ||
| """Alias for :meth:`~screenpy.actor.Actor.exit`.""" | ||
| return self.exit() | ||
|
|
||
| def exit_stage_right(self) -> None: | ||
| """Alias for :meth:`~screenpy.actor.Actor.exit`.""" | ||
| return self.exit() | ||
|
|
||
| def exit_through_vomitorium(self) -> None: | ||
| """Alias for :meth:`~screenpy.actor.Actor.exit`.""" | ||
| return self.exit() |
There was a problem hiding this comment.
Of course these aliases are strictly essential and must be preserved at all costs
In #19, @langgaibo reported that mypy did not like
MakeNote'sof_themethod. After investigating,ofwas OK, butof_thewas causing a complaint. This seems to be a bug, or a shortcoming, or something in MyPy itself, see MyPy#6700.This PR takes the workaround suggested in the above MyPy issue, by defining the aliases as other methods. Technically only the
classmethods had this problem, but i felt it was weird to only do it for those. Doing it for all the aliases allows the documentation to be more explicit about what methods are aliases of which other methods!