Skip to content

Add type hints to vFrequency#1358

Open
arjunrawal1 wants to merge 2 commits into
collective:mainfrom
arjunrawal1:arjun/type-vfrequency
Open

Add type hints to vFrequency#1358
arjunrawal1 wants to merge 2 commits into
collective:mainfrom
arjunrawal1:arjun/type-vfrequency

Conversation

@arjunrawal1
Copy link
Copy Markdown
Contributor

Summary

Refs #938

Testing

  • python3 -m pytest src/icalendar/tests/prop/test_constructors.py src/icalendar/tests/prop/test_unit.py src/icalendar/tests/rfc_7265_jcal/test_invalid_jcal.py -q — 751 passed

Note: I also tried python3 -m mypy src/icalendar/prop/recur/frequency.py, but mypy is not installed in this local environment.

Refs collective#938

Assisted-by: OpenAI Codex
@read-the-docs-community
Copy link
Copy Markdown

Documentation build overview

📚 icalendar | 🛠️ Build #32530250 | 📁 Comparing d2aa8ef against latest (7357cec)

  🔍 Preview build  

4 files changed
± 404.html
± reference/changelog.html
± reference/api/icalendar.prop.recur.frequency.html
± _modules/icalendar/prop/recur/frequency.html

Copy link
Copy Markdown
Member

@stevepiercy stevepiercy 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 the PR! Let me know what you think.


@classmethod
def from_ical(cls, ical):
def from_ical(cls, ical: Any) -> Self:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above.

Suggested change
def from_ical(cls, ical: Any) -> Self:
def from_ical(cls, ical: ICAL_TYPE) -> Self:

cls,
value,
encoding=DEFAULT_ENCODING,
value: Any,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the use of Any is too loose. We have a special variable that you can import. Try this out, and let me know.

Modify the import.

from icalendar.parser_tools import DEFAULT_ENCODING, ICAL_TYPE, to_unicode

Then:

Suggested change
value: Any,
value: ICAL_TYPE,

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