Skip to content

Conversation

@yankyhoffman
Copy link
Contributor

@yankyhoffman yankyhoffman commented Mar 5, 2023

Return an instance of the sub-classed JewishDate type when applying any of the base JewishDate methods (__add__ and __sub__).

Copy link
Owner

@pinnymz pinnymz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finding/fixing this! One minor comment on the test approach, would be happy to merge if you can resolve that.

self.assertEqual(calendar.sof_zman_kiddush_levana_between_moldos().toordinal(), expected_time.toordinal())

def test_date_arithmetic_returns_inherited_type(self):
for instance, type in [(JewishCalendar(5783, 1, 1), JewishCalendar), (JewishDate(5783, 1, 1), JewishDate)]:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to keep other types out of this batch of tests, JewishCalendar alone should suffice here. An alternative approach would be to test the method in TestJewishDate, and create a dummy class to ensure the type isn't being lost.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duly noted, indeed keeps the code-base clean.

@yankyhoffman yankyhoffman requested a review from pinnymz March 6, 2023 15:11
@pinnymz
Copy link
Owner

pinnymz commented Mar 10, 2023

Replaced by PR#15

@pinnymz pinnymz closed this Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants