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

[AMP] Flow de mise à jour des AMP depuis les données OFB #1339

Merged
merged 4 commits into from May 22, 2024

Conversation

thoomasbro
Copy link
Collaborator

Copy link

gitguardian bot commented May 2, 2024

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
9429425 Triggered Generic Password 347a987 datascience/.env.test View secret
9429425 Triggered Generic Password 347a987 datascience/.env.test View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.


@pytest.fixture
def amp_areas_from_ofb() -> gpd.GeoDataFrame:
return gpd.GeoDataFrame(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Il faudrait définir le CRS dans ces test data.

with open(AMP_AREAS_FILE_PATH, "wb") as f:
f.write(BytesIO(r.content).read())

amp_areas = gpd.read_file(AMP_AREAS_FILE_PATH)
Copy link
Collaborator

Choose a reason for hiding this comment

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

J'ai testé cette tâche (avec téléchargement réel du fichier), le GeoDataFrame qui est retourné est en CRS 3857, il faudrait le convertir en 4326 avant de le charger en base (soit ici soit dans le transform), sinon les coordonnées du GeoDataFrame, en m, seront chargées en base telles quelles et y seront interprétées comme des degrés (la table est en CRS 4326)..

>>> from src.pipeline.flows.amp_ofb import extract_amp_areas, AMP_AREAS_URL, PROXIES
>>> gdf = extract_amp_areas.run(url=AMP_AREAS_URL, proxies=PROXIES)
>>> gdf.crs

<Projected CRS: EPSG:3857>
Name: WGS 84 / Pseudo-Mercator
Axis Info [cartesian]:
- X[east]: Easting (metre)
- Y[north]: Northing (metre)
Area of Use:
- name: World between 85.06°S and 85.06°N.
- bounds: (-180.0, -85.06, 180.0, 85.06)
Coordinate Operation:
- name: Popular Visualisation Pseudo-Mercator
- method: Popular Visualisation Pseudo Mercator
Datum: World Geodetic System 1984 ensemble
- Ellipsoid: WGS 84
- Prime Meridian: Greenwich

>>> gdf.geometry

0      POLYGON ((5245262.247 -1335472.553, 5245185.76...
1      POLYGON ((-505860.279 6212614.945, -505906.497...
2      POLYGON ((-493469.154 6215300.894, -494115.212...
3      MULTIPOLYGON (((-6801388.510 1602843.069, -680...
4      POLYGON ((1027730.490 5270046.437, 1028459.692...
                             ...                        
646    POLYGON ((18598387.980 -2551793.628, 18608722....
647    POLYGON ((18481554.173 -2468304.433, 18481521....
648    MULTIPOLYGON (((-485492.783 6088523.295, -4855...
649    MULTIPOLYGON (((-303846.596 6031960.716, -3038...
650    MULTIPOLYGON (((-7015407.702 2050713.523, -701...
Name: geometry, Length: 651, dtype: geometry


@pytest.fixture
def amp_areas_after_upsert() -> gpd.GeoDataFrame:
return gpd.GeoDataFrame(
Copy link
Collaborator

Choose a reason for hiding this comment

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

ici aussi définir le CRS

"ORDER BY mpa_id")


pd.testing.assert_frame_equal(amp_areas_after_upsert, loaded_amp)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comme discuté peut-être essayer gpd.assert_geodataframe_equal pour tester les géométries.

@thoomasbro thoomasbro merged commit 0aab234 into main May 22, 2024
3 checks passed
@thoomasbro thoomasbro deleted the thomas/flow-maj-amp branch May 22, 2024 12:43
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.

None yet

2 participants