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

Enhancement: FWI direct download from EFFIS and GeoJSON conversion #77

Merged
merged 21 commits into from
Feb 19, 2024

Conversation

GHCamille
Copy link
Collaborator

@GHCamille GHCamille commented Nov 3, 2023

This pull request introduces several enhancements to the existing codebase:

  1. Ability to write a JSON file to a s3 locally (using localstack)

  2. FWI downloading through requests package

  3. GeoJSON formatting so that it can be used in Pyronear platform

These changes aims at providing a risk MVP for the platform. There are still some grey areas for me, please MP me after reviewing so that we can go over it !

Please review the changes and provide feedback or approvals as necessary. Thank you!

@juldpnt
Copy link
Collaborator

juldpnt commented Nov 26, 2023

Hello @GHCamille , thanks for this huge PR !
In fwi_helpers.py I don't really understand how fwi_category works:

 categories = [
            (58, 6),
            (145, 1),
            (192, 5),
            (210, 2),
            (231, 4),
        ]

Do you have a link where it is stated how categories are related to their values ? As value(2) > value(4) but value(2) < value(6), it is quite odd to me ?

@Acruve15
Copy link
Collaborator

[QUESTION] Should we keep the current CICD? Seems deprecated (@jsakv)

@jsakv
Copy link
Collaborator

jsakv commented Nov 28, 2023

[QUESTION] Should we keep the current CICD? Seems deprecated (@jsakv)

Yes @Acruve15 the CI/CD is deprecated, multiple directories and modules are also deprecated and some unit tests are calling external API endpoints that are deprecated.

What we can do is to:

  • Add the changes we want to make within the scope of this PR (FWI download and conversion) and merge the PR once we are ok with the code and ignore the CI/CD for now

  • Open a second PR for the refactoring/cleaning of the repository. I have started deleting deprecated files and directories and updating the CI/CD on another branch, once the PR is merged, I can rebase and update the branch raise a PR for review.

Let me know if works for you!

jsakv
jsakv previously approved these changes Dec 2, 2023
@juldpnt juldpnt merged commit 9d057fd into master Feb 19, 2024
3 of 10 checks passed
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.

4 participants