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

Added functions for getting global mortality #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SumilThakr
Copy link
Collaborator

Please take a look.

@SumilThakr SumilThakr requested a review from ctessum March 4, 2020 06:02
Io []float64
}

func loadPopulation(f string) (*rtree.Rtree, map[string]int, error) {

Choose a reason for hiding this comment

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

loadPopulation is unused (from deadcode)

return f, err
}

func loadMortality(f string) ([]*mortality, map[string]int, error) {

Choose a reason for hiding this comment

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

loadMortality is unused (from deadcode)

Copy link
Member

Choose a reason for hiding this comment

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

What I had in mind was more about modifying the functions that were already here to make them more general rather than adding new functions.

Copy link
Member

Choose a reason for hiding this comment

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

Also, these functions can't be used because they're not exported (they don't start with a capital letter).

}

// Load the InMAP concentration output
func loadConc(f string) (*rtree.Rtree, map[string]int, error) {

Choose a reason for hiding this comment

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

loadConc is unused (from deadcode)

}

// regionalIncidence calculates region-averaged underlying incidence rates.
func regionalIncidence(popIndex *rtree.Rtree, popIndices map[string]int, mort []*mortality, mortIndices map[string]int, concIndex *rtree.Rtree, concIndices map[string]int) (*rtree.Rtree, error) {

Choose a reason for hiding this comment

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

regionalIncidence is unused (from deadcode)

if err != nil {
return nil, nil, err
}
switch gg.(type) {

Choose a reason for hiding this comment

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

S1034: assigning the result of this type assertion to a variable (switch gg := gg.(type)) could eliminate the following type assertions:
epi/globalepi.go:88:27 (from gosimple)

if err != nil {
return nil, nil, err
}
switch gg.(type) {

Choose a reason for hiding this comment

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

S1034: assigning the result of this type assertion to a variable (switch gg := gg.(type)) could eliminate the following type assertions:
epi/globalepi.go:160:27 (from gosimple)

if err != nil {
return nil, nil, err
}
switch gg.(type) {

Choose a reason for hiding this comment

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

S1034: assigning the result of this type assertion to a variable (switch gg := gg.(type)) could eliminate the following type assertions:
epi/globalepi.go:224:27 (from gosimple)

return pop, popIndices, nil
}

func s2f(s string) (float64, error) {

Choose a reason for hiding this comment

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

func s2f is unused (from unused)


// inmapOutput holds either population or concentration data read from
// InMAP results
type inmapOutput struct {

Choose a reason for hiding this comment

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

type inmapOutput is unused (from unused)

Data []float64
}

type mortality struct {

Choose a reason for hiding this comment

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

type mortality is unused (from unused)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 68.247% when pulling f73972e on SumilThakr:epiglobal into 0f2c3aa on spatialmodel:master.

Copy link
Member

@ctessum ctessum left a comment

Choose a reason for hiding this comment

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

Thanks for this! I think GlobalGEMM is a good addition. For the other things, we ideally want to be creating functions that are generally usable and don't only apply to the global analysis (so we wouldn't want to call the file "globalepi").

I think the loadPop and loadMort functions are a little too specific (e.g., they assume the data is in shapefile format.)

RegionalIncidence could be a good addition, but I think it would be better if the inputs and outputs were not rtrees (because there's not really any way to tell what's in the rtree), but data arrays or something like that instead. So the inputs would be arrays of population and mortality data (including polygon information) and the output would be regional baseline incidence rates for the mortality shapes.

We will also need unit tests for anything we add.

Comment on lines +68 to +76
// Burnett R, Chen H, Szyszkowicz M, Fann N, Hubbell B, Pope CA III,
// Apte JS, Brauer M, Cohen A, Weichenthal S, Coggins J, Di Q,
// Brunekreef B, Frostad J, Lim SS, Kan H, Walker KD, Thurston GD, Hayes
// RB, Lim CC, Turner MC, Jerrett M, Krewski D, Gapstur SM, Diver WR,
// Ostro B, Goldberg D, Crouse DL, Martin RV, Peters P, Pinault L,
// Tjepkema M, van Donkelaar A, Villeneuve PJ, Miller AB, Yin P, Zhou M,
// Wang L, Janssen NAH, Marra M, Atkinson RW, Tsang H, Thach TQ, Cannon
// JB, Allen RT, Hart JE, Laden F, Cesaroni G, Forastiere F, Weinmayr G,
// Jaensch A, Nagel G, Concin H, Spadaro JV. (2018). Global estimates of
Copy link
Member

Choose a reason for hiding this comment

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

Change to Burnett R et al.

return f, err
}

func loadMortality(f string) ([]*mortality, map[string]int, error) {
Copy link
Member

Choose a reason for hiding this comment

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

What I had in mind was more about modifying the functions that were already here to make them more general rather than adding new functions.

// mortality associated with long-term exposure to outdoor fine
// particulate matter. Proceedings of the National Academy of Sciences:
// DOI: 10.1073/pnas.1803222115.
var GlobalGEMM = Nasari{
Copy link
Member

Choose a reason for hiding this comment

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

Maybe move this so it is next to NasariACS{}.

return f, err
}

func loadMortality(f string) ([]*mortality, map[string]int, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Also, these functions can't be used because they're not exported (they don't start with a capital letter).

@ctessum ctessum force-pushed the master branch 4 times, most recently from afb2b6e to e75e19d Compare August 9, 2021 16:35
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.

4 participants