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

New sica lu api #2766

Merged
merged 4 commits into from
Sep 30, 2024
Merged

New sica lu api #2766

merged 4 commits into from
Sep 30, 2024

Conversation

Foxi352
Copy link
Contributor

@Foxi352 Foxi352 commented Sep 30, 2024

As discussed in #2569 i migrated the sica_lu source to the new API.

Output of test run:

Testing source sica_lu ...
  found 103 entries for Habscht
  found 104 entries for Steinfort

@5ila5 5ila5 merged commit 6e17813 into mampfes:master Sep 30, 2024
2 checks passed
@5ila5
Copy link
Collaborator

5ila5 commented Sep 30, 2024

Thanks for your contribution.

I fixed your type hints:

  • any is not a type, but a python built-in function you need to use typing.Any-
  • your fetch_json might return a list (collections) or a dict (municipalities)

The easiest way to fix this would be to just use Any (or no type hints) there, but I like that my IDE knows what could be in the dict and does some sanity checks for me, so I added these TypedDicts. (maybe run mypy next time to check your types or make sure your IDE does these checks)

and I did some reformatting (black, isort)

You can run all these checks using pre-commit

@Foxi352
Copy link
Contributor Author

Foxi352 commented Oct 1, 2024

Thanks @5ila5 for your fixes, appreciated. I work in cybersecurity and am far from an experienced developer :-)
I am always open to learn something new. Also thanks for the merge.

@Foxi352 Foxi352 deleted the new_sica_lu_api branch October 1, 2024 17:29
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