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

Fix dto=True option in data_assimilation_window_params.py methods #438

Open
Dooruk opened this issue Oct 7, 2024 · 1 comment
Open

Fix dto=True option in data_assimilation_window_params.py methods #438

Dooruk opened this issue Oct 7, 2024 · 1 comment
Assignees
Labels
core development design related issues and improvements

Comments

@Dooruk
Copy link
Collaborator

Dooruk commented Oct 7, 2024

No description provided.

@Dooruk Dooruk added the core development design related issues and improvements label Oct 7, 2024
@Dooruk Dooruk self-assigned this Oct 7, 2024
@Dooruk
Copy link
Collaborator Author

Dooruk commented Oct 11, 2024

Alexey's comment:

This can return either a string or a tuple of a string and a datetime. So, the correct type hint should be something like:

    def local_background_time(self, window_offset, window_type, dto=False) -> Union[str, Tuple[str, datetime]]:

(If we're using Python >= 3.10, then this can be written more concisely as: str | Tuple[str, datetime]).

While I'm pontificating, in the future, I would just drop the dto argument, change this function to always return the background time as the second argument, and then just use tuple unpacking to absorb the unused argument whenever we don't need it.

mystring, _ = self.local_background_time(window_offset, window_type)
# Python will put the datetime into the `_`, which is then silently discarded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core development design related issues and improvements
Projects
None yet
Development

No branches or pull requests

1 participant