Closed Akilditu closed 3 years ago
You cc'd the wrong Florian. :)
You cc'd the wrong Florian. :)
Indeed, sorry for that :)
Thanks for opening the issue @Akilditu ! To follow-up on your questions:
Yup, we're about to merge a PR that creates automatically the super user. After this, we'll create you a login for dev/demo purposes!
On that aspect, would you specify exactly which "fields" do you need for each entry? As of now, our devices, sites and installation tables only have limited fields (cf. https://github.com/pyronear/pyro-api/blob/master/src/app/db/tables.py). Efficiency-wise it would be best to query only one table but then it depends on what you need for the platform!
Hi there,
So far, do you use all the fields of your data ? (as said fg, the db currently has a limited number of fields but it can be developed further)
Tours | Latitude | Longitude | Connexion Alerte CODIS | Caméra en place | Guetteurs Juillet - Août | Nombres Devices | Département |
---|---|---|---|---|---|---|---|
Serre en Don | 44.870959 | 4.395387 | Faisceau | Oui | Non | 1 | 7 |
Serre de pied de Boeuf | 44.758482 | 4.516601 | Faisceau | Non | Non | 2 | 7 |
Serre Barre Gelon 30 | 44.396864 | 4.516601 | 4G | Non | Oui | 5 | 7 |
Tour de Brison Gelon 31 | 44.545155 | 4.216534 | 4G | Non | Oui | 5 | 7 |
@VincGargasson @nrslt @pechouc @Akilditu sorry for the spam, the sooner we know the sooner we can be ready !
Sorry for the late reply, I had completely missed this discussion!
In practice, in our code, we are fetching all the fields for now, but we do not use the whole information. We could completely adapt to what is more practical on your side.
If I am not mistaken, we are displaying only the following data points from the table: Tours
, Latitude
and Longitude
. Additionally, Départements
allows us to show markers only if they are located in the region on which the user has clicked. So, I would say that these 4 fields are needed.
Eventually and if possible, the Nombre Devices
will probably be required too as it is a valuable information to display. Note that this is only my view and the other members of the web-app team might not share it. In any case, if it has been settled that all installations will be composed of 4 cameras, it might not be necessary of course.
Hi guys and sorry for the late reply, I also missed that one.. I agree with @pechouc's answer on Tours
, Latitude
, Longitude
, Départements
. Also, Nombre Devices
and Caméra en place
seem to be valuable information for us too, or am I mistaken on what it means?
Since some lines have Caméra en place
= "Non", though there are multiple devices indicated in the Nombre Devices
column.
Thanks for the early replies!
Regarding Tours, we probably can add a free text field for naming. And we do already have lon & lat. However for "Départements", I'm not sure what to do. To answer this, I took the more global scheme in consideration:
Again this is because, for us, "département" name is a redundant information considering we already have lat & lon. But I understand this is important for the other groups!
Thanks for the reply !
@frgfm, I would say the field name
is available for sites
and okay for the Tours
.
@nrslt, not certain, but I think Caméra en place
states that if there is a camera on site (external to Pyronear project, such as camera used for "doubt removal" ). @Akilditu might be able to confirm.
About Nombres Devices
, this is indeed valuable information. @frgfm it makes me realise, we should link devices to a site. devices could not necessarily related to a site, however, that link could be important, especially for paltform use (from the site, this a way to have quick access to information from related devices). What do you think ?
Then it will be easy to access the number of devices.
Also, I agree with department discussion. (as long as lat/lon are available, might possible to infer postcode fromdepartements.geojson
)
Regarding the name field of sites, it's already deployed: https://github.com/pyronear/pyro-api/blob/master/src/app/db/tables.py#L46
@fe51 but we already have that link through the "installations" table. To get the number of active devices on a specific site, you only need to filter installations whose site_id is the selected ones, with start_ts < current_ts < end_ts. We could add a route to query this, depending on how important that is.
A direct mapping would be better that geojson mapping (as this requires a pandas dependency + some heavy lifting computation-wise)
Indeed my bad 👍 Yes, a specific route might be useful.
Yes sure !
Just opened https://github.com/pyronear/pyro-api/issues/49 and set it as high priority!
Again this is because, for us, "département" name is a redundant information considering we already have lat & lon. But I understand this is important for the other groups!
Yes, of course! I hadn't thought about this but we could totally make a call to a geocoding API or use departements.geojson
to retrieve information from that field, so I see no issues dropping it.
Hi everyone, sorry for the late reply.
@nrslt, not certain, but I think
Caméra en place
states that if there is a camera on site (external to Pyronear project, such as camera used for "doubt removal" ). @Akilditu might be able to confirm.
@fe51 Indeed this refers to others cameras installed on site, mostly there for 'doubt removal' ! This is not a mandatory info but could be interesting in a v2 even if it's not 'pyronear-related' in the way that it's not required by our ecosystem.
Serre Barre Gelon 30 44.396864 4.516601 4G Non Oui 5 7
@fe51 Small mistake here when i created the file, the right long
is 4.083735 and not 4.516601
A direct mapping would be better that geojson mapping (as this requires a pandas dependency + some heavy lifting computation-wise)
@frgfm True so you'll handle that before db insert ?
Also would it be possible to get the azimuth
of a specific device ? I guess this could be a valuable info as well, juste checked and not sure it can be retrieved by yaw
and pitch
right ?
To sum up on this I guess that everything is available except :
postcode
mapping with long latcountry
field addition within installation table (not sure it's needed in 1.0)azimuth
filed (valuable info for firemen mostly considering an 'alert context') Yes, of course! I hadn't thought about this but we could totally make a call to a geocoding API or use departements.geojson to retrieve information from that field, so I see no issues dropping it.
@pechouc My suggestion for the ideal solution would be for us to add the geocode of the département. Because then the resolution of the name would much easier for you guys!
@frgfm True so you'll handle that before db insert ?
@Akilditu I don't believe it's wise, to handle this on the API side. The naming of the département also depends on the selected language which would vary in the future on the platform. If we do handle this on the API, we would only expose a geocode+language--> name route, but for now, perhaps it's best that this is handled on the platform
Also would it be possible to get the azimuth of a specific device ? I guess this could be a valuable info as well, juste checked and not sure it can be retrieved by yaw and pitch right ? Yup yaw is literally that, angle from bird eye view perspective, the pitch is the inclination angle of the camera (thought it could be useful on some setup)
So on your last points:
yaw
field of the device table@pechouc My suggestion for the ideal solution would be for us to add the geocode of the département. Because then the resolution of the name would much easier for you guys!
Ok, perfect! That was just to point at the fact that if useful, we could simply drop the field but the solution you mention is even simpler for us of course. We already have the possibility to make the mapping between the geocode of the département and its name, for example via the departements.geojson
available online.
@Akilditu I don't believe it's wise, to handle this on the API side. The naming of the département also depends on the selected language which would vary in the future on the platform. If we do handle this on the API, we would only expose a geocode+language--> name route, but for now, perhaps it's best that this is handled on the platform
Oh yes agreed, mixed-up between geocode mapping (that could definitely be handle on platform side) and lat lon to postcode
- postcode & country fields will be added by the API team to the "site" table (just opened pyronear/pyro-api#53, set also as high priority
Great
Yup yaw is literally that, angle from bird eye view perspective, the pitch is the inclination angle of the camera (thought it could be useful on some setup
I think that we're all set then, will follow those pyronear/pyro-api#49, pyronear/pyro-api#53 😎
As a follow-up on this I think we could sync to :
define the best way to build requests (doc here right ?)
deal with platform auth (maybe a log-in screen on our side needs to be built, not sure about 1.0 )
As a follow-up on this I think we could sync to :
- define the best way to build requests (doc here right ?)
- deal with platform auth (maybe a log-in screen on our side needs to be built, not sure about 1.0 )
@Akilditu haha, we already have built a python API client for you guys: https://github.com/pyronear/pyro-api/tree/master/client The documentation will be available after we merge an ongoing PR. For now, the client only has methods that were designed for devices needs. On your end, what are the different things you need to do on the API? I can think of:
Is there anything else? Once we have all the needs on your hand, we can already give you the way you will access this, and we'll open a PR to handle this shortly afterwards.
- deal with platform auth (maybe a log-in screen on our side needs to be built, not sure about 1.0 )
Here maybe i was not clear but I mentioned 'auth' for users access to the platform (e.g CODIS)
Is there anything else? Once we have all the needs on your hand, we can already give you the way you will access this, and we'll open a PR to handle this shortly afterwards.
I guess that for 1.0 we'll need to :
query users
to handle auth + retrieve at least logins. Maybe adding a user_group
field would be relevant ('pyro_org' or 'sdis_ardeche')
handle 'live alerting'. I guess that this 'direct alert' feature would be the high priority one to deal with. I'm thinking of creating a proper issue, what's your opinion on that @fe51 , @frgfm ?
Here maybe i was not clear but I mentioned 'auth' for users access to the platform (e.g CODIS)
Oh True, if possible, I suggest we don't put that as mandatory for Wednesday (if possible we'll do it, but we need to first discuss the implementation options between us I guess). I opened an issue for next release over here https://github.com/pyronear/pyro-api/issues/55
Regarding the live alerting, so far we thought about two options with @florianriche:
Either way, opening an issue on the platform repo would be a good idea @Akilditu :+1:
Florian is more knowledgeable than me about this, perhaps he has some insights about what could be done for wednesday!
FYI, I opened https://github.com/pyronear/pyro-api/issues/61 to make sure the API client features are aligned with your needs here
For the platform team: so the corresponding PR (https://github.com/pyronear/pyro-api/pull/62) is still being reviewed, but the client methods that you will need will remain the same as the one proposed in the PR.
I suggested you read installation & usage instructions over here: https://github.com/pyronear/pyro-api/tree/master/client And then the methods you will need on the platform should be the following: https://github.com/pyronear/pyro-api/blob/api_client_v1/client/pyroclient/client.py#L81-L111 (being added by the PR).
You might want to open a PR on your hand to already have this ready and avoid any hurry tomorrow :sweat_smile: Let us know if you need help!
For the platform team: so the corresponding PR (pyronear/pyro-api#62) is still being reviewed, but the client methods that you will need will remain the same as the one proposed in the PR.
I suggested you read installation & usage instructions over here: https://github.com/pyronear/pyro-api/tree/master/client And then the methods you will need on the platform should be the following: https://github.com/pyronear/pyro-api/blob/api_client_v1/client/pyroclient/client.py#L81-L111 (being added by the PR).
You might want to open a PR on your hand to already have this ready and avoid any hurry tomorrow 😅 Let us know if you need help!
Great work guys ! 👍
I'll open a PR tomorrow that will handle requests, could you tell us which creds to use ? (maybe not here though 😅)
Hi API team 👋
The 2 following tasks could be nice first use-cases to enable pyro-api calls from pyro-platform :
user
route to grant an access to the platform --> tests with pyro-platform dev team could be a nice first step and then retrieve the username
as the below 'SDIS Pyrodev' headerinstallation
route to replace theapp/data/cameras.csv
and then create thecamera_positions
DataFrame -->https://github.com/pyronear/pyro-platform/blob/c6a5aa619ecbda619f1cf5c1cf654f9c912d6d13/app/alerts.py#L102 cc @frgfm, @fe51, @jeanpasquier75, @florianriche
Thanks