balglave / FishMap

https://balglave.github.io/FishMap/
Other
0 stars 1 forks source link

[feat] As client, I'd like part 1 of the flat_main to be a documented and tested function #19

Closed statnmap closed 1 year ago

statnmap commented 1 year ago

As client, I'd like part 1 of the flat_main to be a documented and tested function, so that I can guarantee its outputs, whatever changes in the packages

Client - Validation

image ... image

=> https://github.com/balglave/FishMap/blob/main/R/fm_load_data.R

image

=> https://github.com/balglave/FishMap/blob/main/tests/testthat/test-fm_load_data.R

image

=> https://balglave.github.io/FishMap/articles/running-fishmap.html

Dev - Tech

dagousket commented 1 year ago

The refactoring of part1 raised the following warning in the devtools::check()

W  checking whether package ‘FishMap’ can be installed (27.6s)
   Found the following significant warnings:
     Warning: replacing previous import ‘dplyr::union’ by ‘raster::union’ when loading ‘FishMap’
     Warning: replacing previous import ‘dplyr::select’ by ‘raster::select’ when loading ‘FishMap’
     Warning: replacing previous import ‘dplyr::intersect’ by ‘raster::intersect’ when loading ‘FishMap’
     Warning: replacing previous import ‘raster::extract’ by ‘tidyr::extract’ when loading ‘FishMap’

-> this is now fixed

JCasemajor commented 1 year ago

I would suggest putting the setup of the spatial domain (grid_xmin, grid_xmax, grid_ymin, grid_ymax) as parameter for the fm_load_data function as the user will probably change it according to its data (i.e. if we want to study Celtic sea instead of the Bay of Biscay).

@balglave what do you think ?

balglave commented 1 year ago

D'accord avec Juliette. Ces arguments sont necessaires dans 'domain_mesh_spde.R'.

statnmap commented 1 year ago

Merci. Nous avons ouvert un ticket dédié pour ces paramètres particuliers: https://github.com/balglave/FishMap/issues/34
Vous aurez un autre ticket ensuite pour lister tous les autres paramètres que vous jugez nécessaires.

Je ferme le présent ticket si c'est ok pour vous.

JCasemajor commented 1 year ago

Je valide

balglave commented 1 year ago

Je valide