kaneplusplus / bigmemory

126 stars 24 forks source link

changes to attach.resource? #80

Closed phaverty closed 4 years ago

phaverty commented 6 years ago

Hello, I'm debugging some breakage in bigmemoryExtras. It looks like

bigmemory::attach.resources(descriptor, path=dir)

is defunct. Is the new setup stable, or is this still in flux? (I see some commented-out code that looks like this could be an experiment that made it to master early.) It looks like somewhere along the dispatch path, descriptor should pick up dirname and filename elements. Can you provide a bit more info? I'd be happy to finish the changes in bigmemory or to adapt bigmemoryExtras. Please advise.

privefl commented 6 years ago

Function attach.resource is used by attach.big.matrix, what do you mean by defunct? Do you mean it shouldn't be exported?

Dirname and filename are indeed stored in the descriptor so that you can reattach without specifying the backingpath (see https://github.com/kaneplusplus/bigmemory/issues/61).

Not sure I understood all your questions, hope I didn't answer something else.

phaverty commented 6 years ago

Sorry, "defunct" was vague. I meant that the call

attach.resource(descriptor, path)

ignores the "path" argument and goes straight to the method for "big.matrix.descriptor". There is a line in setMethod('attach.resource', signature(obj='big.matrix.descriptor') that used to make the path argument work:

path <- list(...)[['path']]

but it is commented out now.

has the "path" argument to the method for big.matrix.descriptor been disallowed? It sounds like all of the path and filename info should now always be in the descriptor object itself. This is a bit tricky for bigmemoryExtras, as it has its own slots for this info, but I can adapt.

privefl commented 6 years ago

Yeap, when I added the "auto dirname feature", I disabled the path argument, but I left it for backward compatibility.