search

facilebio / FacileAnalysis

Modular & interactive analysis components for bioinformatics
https://facilebio.github.io/FacileAnalysis
Other
17 stars 4 forks source link

Assert that dims >= 3 first? #10

Closed tomsing1 closed 5 years ago

tomsing1 commented 5 years ago

https://github.com/facileverse/FacileAnalysis/blob/f7ef522e57393dd4ff34fa06220ffa4f780c5048/R/fpca.R#L54

I seems that dims must be >= 3. Perhaps this would be a good place for an assertion that alerts the user of this fact? (It also seems that the arguments for the fpca methods are not yet documented.)

lianos commented 5 years ago

I'm not sure if (or when) code is run at the generic-definition (the line that you are pointing to).

This assertion is tested, though, down in the fpca.matrix function definition, though. The upside is that there is an explicit check for that, but the down side is that all of the necessary has been instantiated and loaded into memory before the assertion takes place.

Maybe it's worth moving this up to the fpca.facile_frame, fpca.FacileDataSet (and similar methods) to avoid that?

As for documentation: noted. I'll close this issue when I elaborate on them a bit.

tomsing1 commented 5 years ago

Right! Yes, you already have assertions, but I am always a bit sad when a trivial assertion is only run after loading data and preparing for lots of work. Just my preference, though.

lianos commented 5 years ago

I agree.

1c041c0a31cf5f619eb042af234c7cd6334464da adds the assertion/check to the methods that "lazily" wait for the data to be loaded (the facile ones). The fpca.DGEList, fpca.EList, etc. rely on this check in the fpca.matrix function since the data is already in RAM. Also added some more fpca docs.