Skip to content

Conversation

@Conengmo
Copy link
Member

@Conengmo Conengmo commented Oct 16, 2023

  • Add type hints to all modules in Branca.
  • Make sure Mypy passes.

Contains some (internally) functional changes as well:

  • Alter _parse_hex to output RGBA floats directly. We only use that function inside of _parse_color, which needs RGBA floats as output.
  • No longer have bytes as accepted type for color strings. That's a remnant from the Python 2 times.
  • Merge duplicate code in Link, JavascriptLink and CssLink.

Do separately later:

  • make subclasses of Figure use the same types for width and height. Also a separate PR I think. Would be easiest to have _parse_size just return a single string.
  • simplify the _camelify function. Is it even needed?
  • remove get_templates

@Conengmo Conengmo marked this pull request as ready for review October 16, 2023 14:11
@Conengmo Conengmo changed the title Add type hints [WIP] Add type hints Apr 4, 2024
@Conengmo
Copy link
Member Author

Conengmo commented May 6, 2024

@ocefpaf would you mind taking a look at this PR at some time? No hurry. I know it's pretty big, but I don't know how to do it otherwise, it's just a lot of type hints. I'm open to suggestions! I'm asking you since you also did python-visualization/folium#1677.

@ocefpaf
Copy link
Member

ocefpaf commented May 7, 2024

@ocefpaf would you mind taking a look at this PR at some time? No hurry. I know it's pretty big, but I don't know how to do it otherwise, it's just a lot of type hints. I'm open to suggestions! I'm asking you since you also did python-visualization/folium#1677.

Sure. I don't have a lot of experience but we can try to run mypy here to check things? I'll see what we can do. There are also some pre-commits that can help.

PS: we need to fix a conflict but I'm looking at it now.

- name: Mypy test
shell: bash -l {0}
run: |
mypy branca
Copy link
Member

Choose a reason for hiding this comment

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

Ha! You are always one (many actually) step ahead of me.

@Conengmo
Copy link
Member Author

Conengmo commented May 7, 2024

Thanks Filipe, much appreciated! I’ll solve the merge conflict before merging. Good idea to add a precommit config as well, I’ll do that in a separate PR.

@Conengmo Conengmo merged commit 8a8a214 into python-visualization:main May 18, 2024
@Conengmo Conengmo deleted the type-hints branch May 18, 2024 09:00
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