Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Eliot McIntire
    @eliotmcintire
    This message was deleted
    Eliot McIntire
    @eliotmcintire
    Welcome! We are experimenting with this gitter app for disucssions about the SpaDES-core package in R. For immediate issues, please create an issue on GitHub.com/PredictiveEcology/SpaDES-core
    Eliot McIntire
    @eliotmcintire
    @achubaty I have pushed a PR with the work around. It doesn't not identify the line in Copy that causes the problem.
    Eliot McIntire
    @eliotmcintire

    To reproduce problem with debugging, checkout SpaDES.core@ShowEnvironmentError

    devtools::test(filter = "checkpoint$")

    That will open a browser() call and then the next 5 lines or so can be run interactively... the all.equal is TRUE, then change the environment and it goes all weird.

    Eliot McIntire
    @eliotmcintire
    Should there be a check during each do.event call at the SpaDES.core level for "enormous" objects that shouldn't be, like formula and function classes?
    ianmseddy
    @ianmseddy

    I'm not sure I like the solution to this name collision issue. I'm posting the example here instead of scfm because it could happen with anything, really. In this case, one object (from scfm) is a spatialPolygonsDataFrame, and the other is a list (whatever Tati was referring to, presumably fireSense_dataPrep). I'm not sure why she needs both objects in the sim list, but that's irrelevant. In this case, the fix was this:

     if (is(sim$firePoints, "list")){
        tmp <- do.call(what = bind, args = list(sim$firePoints))
      } else {
        tmp <- sim$firePoints
      }
    
    # sim$firePoints <- tmp # TM ~ Terrible terrible idea. Other modules might use this object and
      # espect it to be the list of fire points
      # You shouldn't modify it unless its a dataPrep module or smth and even so, its dangerous.
      # Just use your local variable, or save as a new sim.Object if this needs to go across modules.
      # I checked all SCFM modules, and only regime uses firePoints, so I am keeping the local
      # Object
    #several lines later 
      # firePolys <- unlist(sim$firePoints) # TM ~ Use tmp, as its already unlisted!
      firePolys <- tmp # TM ~ Use tmp, as its already unlisted!

    sim$firePoints could be anything supplied by another module, including objects that can't be coerced into something usable by 'scfm', unlike in this example. So while the workaround is correct, I don't think the code that does this should happen in scfm. A user who only uses scfm might wonder "why can firePoints be a list if the metadata stipulates SPDF, and the module supplies an SPDF". It feels like an integration module should be used instead, but this is tricky because presumably both modules using 'sim$firePoints' are parameterization modules that need it during some init event. So whatever integration module is created, it would have to schedule this conversion to happen in between the two init events. But either way it is an idiosyncratic integration issue that belongs in an idiosyncratic integration module. And even that doesn't resolve the general problem of having two completely irreconcilable objects with the same name. That's my take anyway.

    Eliot McIntire
    @eliotmcintire
    Like you, I don't like this solution. I feel like a possibility is for the module developers who use this object to talk and agree on a single object type/class. It would involve some code changing but it would be better than this solution which is trying to be more than one class/type.
    Alex Chubaty
    @achubaty
    i agree -- in this particular case we have the power to change the name of the object so that's what we should do.
    Eliot McIntire
    @eliotmcintire
    After discussion, it seems like the best solution to this specific case is to rename both objects, with slightly more precise names.