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

[add] print method for neuronlistz #468

Merged
merged 5 commits into from
Jun 12, 2021
Merged

Conversation

dokato
Copy link
Contributor

@dokato dokato commented Jun 10, 2021

Addresses #466 .

I simplified so it doesn't call expensive contents.classes <- sapply(x, firstclass, USE.NAMES = F).

Artefact, we don't print the information about neuron types.

> nzmv
'neuronlistz' containing 3000 neurons objects and 'data.frame' with 1 vars [998 kB]

Benchmark on 3000 neurons:

# old
> system.time(print(nzmv))
'neuronlistz' containing 3000 'neuprintneuron' objects and 'data.frame' with 1 vars [998 kB]
   user  system elapsed 
 16.702   1.805  19.000 
# new
> system.time(print(nzmv))
'neuronlistz' containing 3000 neurons objects and 'data.frame' with 1 vars [998 kB]
   user  system elapsed 
  0.000   0.000   0.001 

Alternatively, we can add another rds that stores that info in zip? WDYT @jefferis ?

@coveralls
Copy link

coveralls commented Jun 10, 2021

Coverage Status

Coverage increased (+0.04%) to 81.425% when pulling 0bc9ef3 on dokato:printneuronlistz into 88507a8 on natverse:master.

@jefferis
Copy link
Collaborator

Thanks @dokato. This is good, but can you add the path to the zip file and its size on disk to the displayed information and make it clear that the currently printed size is just for the portion in memory?

@dokato
Copy link
Contributor Author

dokato commented Jun 11, 2021

Is message like this okay?

'neuronlistz' containing 3000 neurons objects and 'data.frame' with 1 vars.
Neurons loaded on demand from 'testz.zip' [93.1 MB]. Currently in memory: 998 kB.

@jefferis
Copy link
Collaborator

Thanks! How about:

'neuronlistz' containing 3000 neurons objects and 'data.frame' with 1 vars [998 kB in RAM].
Loaded on demand from '/x/y/z/testz.zip' [93.1 MB on disk].

@dokato
Copy link
Contributor Author

dokato commented Jun 11, 2021

Sure, done!

@jefferis jefferis merged commit a155984 into natverse:master Jun 12, 2021
@jefferis
Copy link
Collaborator

Thanks again @dokato!

@dokato dokato deleted the printneuronlistz branch April 21, 2022 14:36
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.

3 participants