12rambau / sepal_ui Goto Github PK

Using the LSIB 2017 description of countries is perfectly fine but some countries have common description numbers. We should use another system to describe AOI selected by country name like ISO_3166-1_alpha-3 country codes

Local raster images are not being displayed in voilá dashboards nor Jupyter SEPAL dashboards, the problem comes up when the xarray_leaflet module is stating the base_url, using the 'notebooks' word.

The sepal module dashboards are being stored in /shared/notebooks/, so the condition is trying to find the last 'notebook' word, but as there are two occurrences in the URL, it will take the second, causing confusion and pointing to nowhere.

Three solutions come up to my mind:

• Post to xarray-leaflet and try to change the way to get the base URL (could be using the first occurrence instead of the last).
• Move the SEPAL dashboards to another folder
• Change the structure in the xarray-leaflet module by forking it.

As the UI starts to be used in several modules that are available on the dashboard we should create some automatic tests to verify that the modification we implement don't break already existing components

In the Alert class, we are adding a message to che object children.

For output that always display the same message (gee interactions for example) I think we could add the timestamp to show the end user that the process is still running :

here is a proposal solution :

def add_msg(self, msg, type_='info'):
        self.show()
        self.type = type_ if (type_ in self.TYPES) else self.TYPES[0]
        self.children = [msg]

def add_output(self, msg, type_'info'):
    current_time = datetime.now().strftime("%Y/%m/%d, %H:%M:%S")

    self.show()
    self.type = type_ if (type_ in self.TYPES) else self.TYPES[0]
    self.children = [
        v.Html(tag='p', children=['[{}]'.format(current_time)]),
        v.Html(tag='p', children=[msg])
   ]

File aoi.py has 314 lines of code (exceeds 250 allowed). Consider refactoring.

https://codeclimate.com/github/12rambau/sepal_ui/sepal_ui/aoi.py#issue_5f8f0e3fd549d5000100004e

In all the apps the fileinput widget sucks as it's pointed to the local folder of the computer

The txt file dropdown is not a good workaround as people have too much file in there sepal folders.

Whenever I try to test geemap instances, I'm redirected to tha google authentification which is not possible in travis build.

I was thinking that the sepal_ui.scripts.utils.init_ee() function was enough of a workaround but geemap has it's own way of authenticating in earthengine.

asked in geemap repository : gee-community/geemap#189

When the aoi is pulled from gee assets, they always come with the same patterned name. would be cool if we can retrieve this name automatically

such as

"aoi_myaoi" => "myaoi"

The functions in mapping.py are useful but could be more useful if they were wrapped in a python Map object

The default datepicker is difficult to use and we always use it the same way. Could it be useful to add it as a default sepal widget ?

def update_map(self, assetId, bounds, remove_last=False):
        """Update the map with the asset overlay and removing the selected drawing controls
        
        Args:
            assetId (str): the asset ID in gee assets
            bounds (list of tuple(x,y)): coordinates of tl, bl, tr, br points
            remove_last (boolean) (optional): Remove the last layer (if there is one) before 
                                                updating the map
        """  
        if remove_last:
            self.remove_last_layer()

        self.set_zoom(bounds, zoom_out=2)
        self.centerObject(ee.FeatureCollection(assetId), zoom=self.zoom)
        self.addLayer(ee.FeatureCollection(assetId), {'color': 'green'}, name='aoi')


    def set_zoom(self, bounds, zoom_out=1):
        """ Get the proper zoom to the given bounds.

        Args:

            bounds (list of tuple(x,y)): coordinates of tl, bl, tr, br points
            zoom_out (int) (optional): Zoom out the bounding zoom

        Returns:

            zoom (int): Zoom for the given ee_asset
        """

        tl, bl, tr, br = bounds
        
        maxsize = max(haversine(tl, br), haversine(bl, tr))
        lg = 40075 #number of displayed km at zoom 1
        zoom = 1
        while lg > maxsize:
            zoom += 1
            lg /= 2

        if zoom_out > zoom:
            zoom_out = zoom - 1

        self.zoom = zoom-zoom_out

I don't get why you gave bounds in AND assetId in updateZoom. Is it linked to a specific application ? because if you have the assetId you can easily compute the bounds ad if you want to zoom on something else you shoud not give the assetId.

Would you mind if I create two zooming method, one manual with bounds and zoom_out and an autmatic with assetId and an optional zoom_out ?

def centerAsset(assetId, zoom_out= 1):

    return self

def centerBounds(bounds, zoom_out=1):
    
    return self 

geemap is a child class of both ipyleaflet and folium so we don't need to import any of these two object function

more importantly, the basemaps from leaflet are all natively supported like this :

m.add_basemap('CartoDB.Positron')

Local raster can usedirectly satellites bands and need to be stretched before being displayed. It could be interesting to add a stretching options with differents possiblity.

Add in the comment the oft command that you would like to see in sepal_ui

Updated list of the oft functions in sepal_ui

• oft-clump
• oft-his

I want to check a pts variable before launching a process.

if not wb.checkInput(pts, output, ms.NO_PTS): return su.toggleLoading(widget)

with pts being a pandas dataframe.

I raised the following error :

ValueError: The truth value of a DataFrame is ambiguous. Use a.empty, a.bool(), a.item(), a.any() or a.all().

I realized that sepal_ui does not integrate so nicely in the rest of the sepal app as it's not using the same color scheme.

A good starting point would be to use the dark theme as default behaviour

what do you think ?

implementation

to do it we need to add the :
v.theme = dark

and ask Daniel to use the ?voila-theme=dark option in the url
We tested it and it worked

Function add_raster has a Cognitive Complexity of 14 (exceeds 5 allowed). Consider refactoring.

https://codeclimate.com/github/12rambau/sepal_ui/sepal_ui/mapping.py#issue_5f8f0e3fd549d5000100005c

in the current implementation of Btn, only 'mdi-adjust' and 'mdi-download' are supported.
I suggest we let the user decide in the mdi lib which icon he wants to use.

In any case if you write an icon that does not exist ipyvuetify will display an empty square.

Function run_aoi_selection has a Cognitive Complexity of 22 (exceeds 5 allowed). Consider refactoring.

https://codeclimate.com/github/12rambau/sepal_ui/sepal_ui/scripts/run_aoi_selection.py#issue_5f8f0e3fd549d50001000039

Some Widget required the d-inline html class to be displayed. d-none and d-inline cannot leave in the same class_ attribute if we want the hide() method to work

Would be nice if the user could use the current geemap inspector to get the pixel value of the raster image, this function is useful to inspect a raster without the need of moving to a desktop app.

Some elements have a mt-5 class but not all of them. So I suggest we place the classes in each different objects

The aoi_selector options may not all be useful for every module.
A constraint option that could restraint the available method could be handful

The map are completely out of control when you move from one page to another so It would be better if the drawer activation trigger a resize event.

Function bind_aoi_method has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.

https://codeclimate.com/github/12rambau/sepal_ui/sepal_ui/aoi.py#issue_5f8f0e3fd549d5000100004f

Is it a normal behaviour ?

The user may need to use points as an AOI.

• It would require the aoi_selector to create a ee.Geometry.MultiPoint base on a csv file.

If we want to continue to use the asset internally in sepal it could be useful to offer the possiblity to create a shapefile from the aoi geometry

referenced in gee-community/geemap#143

Lots of the computation performed with python in sepal implies an interaction between gee (and gdrive) and the sepal folder.

Do you think it could be useful to create standard drive_handler object ? (inspired by the one in pysmm and glad_alert)

with some standard method like

• get_items
• download to sepal
• print list
• get file id
• delete file
  ...

If some application requires specific implementation you can just create your custom drive_handler inheriting from this one

In the aoi selector, the assets need to be written by the user. It should be possible to select my own aoi with a dropdown selector.

Every git repository used to create a sepal app starts with the same things (a tree, a ui, a no_ui, a about section, a disclaimer, a gitignore, a licence) lets make it automatic with a app skeleton that you can launch when you start a new app

For sementic reason I disagree with the way Alert.clear is designed :

def clear(self):
        self.hide()
        self.children = ['']

here the method hide AND clear. I think we must keep these methods separated and for clarity in codes do them manually

Add in the comment the gdal command that you would like to see in sepal_ui
Remember that a lot of them are already bind in osgeo.gdal

Updated list of the oft functions in sepal_ui

• gdal_merge.py
• gdal_calc.py

Travis CI decided to change its pricing model this week.

I only have 10000 jobs left before I need to pay for extra tests. This is there worse move as every open-source project will move to free billing services.

Anyway apparently the best alternative is to go to Github Actions, I'll look into int next week.

Since the add of the styles.py file, the drawer is not hight enought to overlay the toolbar on small screen. The direct consequence is that the first line of the "menus" is hidden to the end user.

Needs to find a way to deal with both the assets (generated when selecting a manual country for example) and the country that does not require to be exported.

Every time you start the module, it creates a Downloadsfolder with a capital D
I think it's coming from geemap as it's a default folder in the other OS (I mean not linux).

It conflicts with the default downloads folder of sepal, we must find a way to remove it or prevent it's creation

The computation performed by our modules need to be based on the latest version of the FAO GAUL level 0 country boundaries.

ee.FeatureCollection("FAO/GAUL/2015/level0")

when reading a shapefile, only the geometry is loaded.
It should be possible to load all the features in the asset

For some module the developer could have let a custom tutorial that require the use of a specific aoi. let it be declared in the _init of the aoi

The only we can confirm to the end user the value that he has selected with the inputs is to use the alert.
This confirmation is (for me) important as some widgets can have slow behaviour (dropdowns and slider mainly).

at the moment the binding is performed with the function bind in widgetBinding module.

I suggest to add the bind function as a method of the Alert object of SepalWidget as such :

def bind(self, input, obj, variable, output_message='The selected variable is: '):

    #create the on_event litener

    #bind the obj variable to the input v_model

    #display it in the alert

    return 

what do you think ?

As a member method of Aoi_io, why the get_boundsmethod is not using the assetId value ?

def get_bounds(self, ee_asset=None, cardinal=False):
        """ Returns the min(lon,lat) and max(lon, lat) from the given asset

        Args:
            ee_asset (ee.object): GEE asset (FeatureCollection, Geometry)
            cardinal (boolean) (optional)

        Returns:
            If cardinal True: returns cardinal points tl, bl, tr, br
            If cardinal False: returns bounding box
        """

        # 
        ee_bounds = ee.FeatureCollection(ee_asset).geometry().bounds().coordinates()
        coords = ee_bounds.get(0).getInfo()
        ll, ur = coords[0], coords[2]

        # Get the bounding box
        min_lon, min_lat, max_lon, max_lat = ll[0], ll[1], ur[0], ur[1]


        # Get (x, y) of the 4 cardinal points
        tl = (min_lon, max_lat)
        bl = (min_lon, min_lat)
        tr = (max_lon, max_lat)
        br = (max_lon, min_lat)

        return (tl, bl, tr, br) if cardinal else (min_lon, min_lat, max_lon, max_lat)