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

Add date/time prompt #178

Open
wants to merge 50 commits into
base: master
Choose a base branch
from
Open

Add date/time prompt #178

wants to merge 50 commits into from

Conversation

NicDom
Copy link
Contributor

@NicDom NicDom commented Nov 2, 2021

What is the problem that this PR addresses?

  • Fix Add a date/time input #121
    • Add Date and Time prompts via date function
    • Add default validation and completion of text inputs for dates
    • Allows user to easily use own or third-party-library date parsers for validation and completion
    • Own, customized validators and completers can also be used
  • Add testsuite for new module
  • Add new feature to documentation
  • Add examples in documentation

...

How did you solve it?
Implemented date function, that returns a Question regarding a date/time input.
Prompting for dates/times is realized similar to path prompts:
Dates/times are to be entered via a text input, while the user consecutively offered different completions.

Two basic methods of completion and validation are implemented, to which I refer as the simple and parsing ones.

The simple one depends on a given date_format consisting of datetime code formats, e.g. %Y-%m-%d, and is expecting the user input to follow that format. It then offers completions according to the chosen format and input and validates the input using the format.
Note that currently only certain date_formats are supported. If a not supported one is entered, it raises a ValueError.

The parsing one performs validation and completion using a given parser. If the parser is able to parse the input into a date, the input is considered to be valid and the parsed date-object is offered as a completion.
The date-module implemented date-parser (custom_date_parser) is by default used in the parsing method. Own parsers or third-part-libraries can be used.

Both completion and validation methods, the simple and parsing one, are then combined in the date-function to one
method, referred as full on. This method is the default method used for completion and validation.

A typical would look like this:

>>> import questionary
>>> questionary.date("Type a date: ").ask()
# ? Type a date: 2021-01-01
# '2021-01-01'

or, for example using dateutil for 'parsing' validation and completion

>>> import questionary
>>> import dateutil.parser
>>> questionary.date("Type a date or time: ", parser=dateutil.parser.parse).ask()
# ? Type a date or time:  2021 jan 1
# '2021-01-01 00:00:00'

...

Checklist

  • I have read the Contributor's Guide.
  • I will check that all automated PR checks pass before the PR gets reviewed.

✨ add a simple date completer
✅ add various tests for 'SimpleDateCompleter'
📝 updated documentation according to new default parser'

📝 added examples to 'custom_date_parser'
@NicDom NicDom changed the title Feature/date Add date/time prompt Nov 2, 2021
@kiancross
Copy link
Collaborator

Hi @NicDom. This looks excellent - thank you! I won't have time to review this until December, but having briefly tried out the new date input and had a quick scan of the code, there should not be many issues :)

@tmbo
Copy link
Owner

tmbo commented Dec 19, 2021

@NicDom thanks a lot for your contribution - will give it a review 🔥 (sorry for the delay, things have been crazy at work)

Copy link
Collaborator

@kiancross kiancross left a comment

Choose a reason for hiding this comment

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

Thanks again for this PR. Some comments below.

  • Unless they are needed for this PR, can you remove the changes to poetry.lock.
  • The date function needs adding to the top-level __init__.py so that it can be accessed from the API.

README.md Outdated Show resolved Hide resolved
Comment on lines +86 to +88
DAY = [str(i) for i in range(1, 32)]
MONTH = [str(i) for i in range(1, 13)]
YEAR = ["0" * (4 - len(str(i))) + str(i) for i in range(9999)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the DAY autocompletions need to be dependent on the month and year. Some months have different numbers of days, and the YEAR determines whether there it was a leap year or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @kiancross,

yeah you are right. I'll guess the easiest way is to make the completer check, if a completion may lead to a valid date or not before yielding it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi,

ok, I added a validation of the completions before yielding it via adding the following method to SimpleDateCompleter:

def _valid_completion(self, completion: str) -> bool:
        """Checks if a completion is a valid date.

        Used in :function: `get_completion` to reduce all options for completions
        to only those that lead to a valid date.
        Similar to :function: `custom_date_parser`, but does not ignore delimeters.

        Args:
            completion (str): The completion-option that is to be checked, if it
            leads to an valid date.
        """
        format_as_list = self.format.split(self.delimeter)
        date_formats = [
            self.delimeter.join(format_as_list[0:i])
            for i in range(1, len(format_as_list) + 1)
        ]

        # add delimeter to all but the last entry
        for i in range(len(date_formats) - 1):
            date_formats[i] += self.delimeter

        def _try_date_format(
            date_format: str, text: str
        ) -> Optional[datetime.datetime]:
            """Tries to parse ``text`to :class: `datetime.datetime`."""
            try:
                return datetime.datetime.strptime(text, date_format)
            except Exception:
                return None

        # try parsing for the several date_formats
        for date_format in date_formats:
            _date = _try_date_format(date_format=date_format, text=completion)
            # if parsing succeeded return True
            if _date is not None:
                return True
        # no parsing succeeded so return False
        return False

_valid_completion is then called before yielding the completion:

# if completion leads to a valid date, yield this completion
if self._valid_completion(full_output):
    yield Completion(
        full_output,
        start_position=-len(document.text),
        style="fg:ansigreen bold",
        selected_style="fg:white bg:ansired bold",
        display=completion,
    )

I'll now use/introduce style classes for style and selected_style to make them customizable.

Comment on lines 233 to 234
style="fg:ansigreen bold",
selected_style="fg:white bg:ansired bold",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can these styles be made to use one of the classes (or add a new class) so that they can be customised.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
style="fg:ansigreen bold",
selected_style="fg:white bg:ansired bold",
style="class:options",
selected_style="class:selected",

where

("options", "fg: ansigreen bold"),  # options given by autocomplete

Moreover, I introduced a default style for selected in date, which differs from the one in DEFAULT_STYLE:

# define a default style for selected completions. Gets overwritten if another
# format for selected is given by `style`
default_date_styles = Style([("selected", "fg: white bg: ansired bold")])

merged_style = merge_styles([DEFAULT_STYLE, default_date_styles, style])

questionary/prompts/date.py Outdated Show resolved Hide resolved
questionary/prompts/date.py Outdated Show resolved Hide resolved
questionary/prompts/date.py Outdated Show resolved Hide resolved
questionary/prompts/date.py Outdated Show resolved Hide resolved

if print_date_format:
rprompt = to_formatted_text(
f"Date format: {date_format}", style="bg: ansigreen bold"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can these styles use a class (or a preexisting class) so that they can be customised.

Copy link
Contributor Author

@NicDom NicDom Dec 27, 2021

Choose a reason for hiding this comment

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

Suggested change
f"Date format: {date_format}", style="bg: ansigreen bold"
if print_date_format:
rprompt = to_formatted_text(
f"Date format: {date_format}", style="class:information"
)

with

(
    "information",
    "bg: ansigreen bold",
),  # information displayed at the lower right

defined in DEFAULT_STYLE in constants.py.

@NicDom
Copy link
Contributor Author

NicDom commented Dec 22, 2021

Hi @tmbo and @kiancross,

thanks for reviewing the PR. I will work on your suggested changes as soon as I can.

NicDom and others added 7 commits December 22, 2021 17:49
Co-authored-by: Kian Cross <kian@kiancross.co.uk>
Fix typo

Co-authored-by: Kian Cross <kian@kiancross.co.uk>
Fix typo

Co-authored-by: Kian Cross <kian@kiancross.co.uk>
Fix typo

Co-authored-by: Kian Cross <kian@kiancross.co.uk>
Fix typo

Co-authored-by: Kian Cross <kian@kiancross.co.uk>
Completions are checked, if the lead to a valid date
@tmbo
Copy link
Owner

tmbo commented Dec 25, 2021

Thanks a lot @NicDom how are things going, ready for another review? 🙂

@NicDom
Copy link
Contributor Author

NicDom commented Dec 27, 2021

Hi,

sorry, have been "offline" the last days. I am going to look in to your suggested changes today.

@NicDom
Copy link
Contributor Author

NicDom commented Jan 14, 2022

Hi,
Have you had time to take a look at the changes? 🙂

@NicDom
Copy link
Contributor Author

NicDom commented Aug 8, 2022

Hey,
how are things going? How do you feel about the changes?

@kiancross
Copy link
Collaborator

Hi @NicDom - I can take a look on the weekend.

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.

Add a date/time input
3 participants