Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary or self._ret_type #107

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

evanpurkhiser
Copy link
Contributor

We already do this in the _get_next function body here

ret_type = ret_type or self._ret_type

@kiorky
Copy link
Owner

kiorky commented Oct 30, 2024

no, self._ret_type can be datetime, so it will be used.

@kiorky kiorky closed this Oct 30, 2024
@evanpurkhiser
Copy link
Contributor Author

@kiorky Could you take a look at this again. I am not changing any functionality here.

The change is that these calls were passing

ret_type or self._ret_type

as the argument to ret_type of _get_next.

But that is unnecessary, we can just pass through the provided ret_type to the _get_next call. Even if it is None, this line in the _get_next will always default back to the self._ret_type if it was None.

ret_type = ret_type or self._ret_type

I am not removing functionality I am just simplifying the code.

@kiorky
Copy link
Owner

kiorky commented Oct 30, 2024

@kiorky Could you take a look at this again. I am not changing any functionality here.

The change is that these calls were passing

ret_type or self._ret_type

as the argument to ret_type of _get_next.

But that is unnecessary, we can just pass through the provided ret_type to the _get_next call. Even if it is None, this line in the _get_next will always default back to the self._ret_type if it was None.

ret_type = ret_type or self._ret_type

I am not removing functionality I am just simplifying the code.

i know, this is because the code has already been simplified and all those methods are now proxies of _get_next, but that was not always the case in earlier versions of croniter, and the idea would be to let the possibility for proxies not to have all the same behavior.

Well, let's simplify, that can be changed if we need it again.

@kiorky kiorky reopened this Oct 30, 2024
@kiorky kiorky merged commit 7bc3d62 into kiorky:master Oct 30, 2024
8 checks passed
@kiorky
Copy link
Owner

kiorky commented Oct 30, 2024

Thx again again again again again again again again again again again again for the contribution !

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