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

rewrite function get_altitude_intersection #10

Merged
merged 5 commits into from
Oct 23, 2024

Conversation

jbrieuclp
Copy link
Contributor

@codecov
Copy link

codecov bot commented Apr 20, 2023

Codecov Report

Patch coverage is 80.00% of modified lines.

Files Changed Coverage
...ac7036dd9_rewrite_fct_get_altitude_intersection.py 80.00%

📢 Thoughts on this report? Let us know!.

Copy link
Contributor

@jpm-cbna jpm-cbna left a comment

Choose a reason for hiding this comment

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

A priori, il faudrait reformater le fichier avec Black car le check correspondant de la PR ne passe pas : https://github.com/PnX-SI/RefGeo/pull/10/checks

J'ai juste rajouter un commentaire concernant la simplification et l'amélioration de la lecture du SQL (forme) mais sinon tout me semble ok sur le fond.

Comment on lines 45 to 72
-- Use dem
RETURN QUERY
SELECT min((altitude).val)::integer AS altitude_min, max((altitude).val)::integer AS altitude_max
FROM (
SELECT public.ST_Intersection(
rast,
public.ST_Transform(myGeom, thesrid)
) as altitude
FROM ref_geo.dem AS altitude
WHERE public.ST_Intersects(rast,public.ST_Transform(myGeom,thesrid))
) AS a;
-- Use dem_vector
ELSE
RETURN QUERY
WITH d as (
SELECT public.ST_Transform(myGeom,thesrid) a
)
SELECT min(val)::int as altitude_min, max(val)::int as altitude_max
FROM ref_geo.dem_vector, d
WHERE public.ST_Intersects(a,geom);
Copy link
Contributor

Choose a reason for hiding this comment

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

Je ne suis pas certain que tous les alias présent dans les requêtes de cette section de code soit vraiment nécessaire. Cela vaudrait le coup de les supprimer s'ils sont inutiles afin de simplifier les requêtes.

Au passage, passer tous les "as" restant en majuscule "AS" serait mieux.

@mvergez
Copy link

mvergez commented Jun 22, 2023

J'ai testé et ça fonctionne super ! Merci beaucoup !

Serait-il possible de passer black pour formatter le fichier de migration ?

LIMIT 1
INTO is_vectorized;

IF is_vectorized IS NULL THEN
Copy link
Contributor

Choose a reason for hiding this comment

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

Voir la possibilité d'ajouter un sous cas lorsque les géometries sont de type point :

  IF is_vectorized IS NULL AND st_geometrytype(myGeom) = 'ST_Point' THEN
        RETURN QUERY
        WITH alt AS (
            SELECT st_value(rast, public.st_transform(myGeom,thesrid))::int altitude
            FROM ref_geo.dem AS altitude
            WHERE public.st_intersects(rast,public.st_transform(myGeom,thesrid))
        )
        SELECT min(altitude) AS altitude_min, max(altitude)  AS altitude_max
        FROM alt;
   ELSIF is_vectorized IS NULL THEN

Copy link
Member

Choose a reason for hiding this comment

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

Oui, intéressant. A priori bien plus performant ?

@amandine-sahl amandine-sahl force-pushed the rewrite-get-altitude-function branch from 6c386eb to f459a48 Compare September 18, 2023 09:29
@jacquesfize jacquesfize force-pushed the develop branch 2 times, most recently from 72bdde3 to 74617d9 Compare January 29, 2024 15:44
@jacquesfize jacquesfize force-pushed the rewrite-get-altitude-function branch from f459a48 to 150f793 Compare October 23, 2024 06:42
@jacquesfize jacquesfize merged commit 5436b3d into PnX-SI:develop Oct 23, 2024
3 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.

6 participants