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

Origin/decoding #803

Open
wants to merge 253 commits into
base: master
Choose a base branch
from
Open

Origin/decoding #803

wants to merge 253 commits into from

Conversation

farznaj
Copy link
Contributor

@farznaj farznaj commented Feb 28, 2022

No description provided.

Farzaneh Najafi and others added 30 commits June 16, 2020 19:36
…pth has issues; adding depth/area color coding to scatter plots
…se them for umap analysis; also removing all checkpoint files.
…opying them, neuropil subtraction, dff computation
a_data_now = a_data[a_data['area'].values==ars]
# a_data_now = a_data

### ANOVA
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should remove outdated, commented-out code

####################################################################################################
####################################################################################################

def add_tukey_lines(tukey_all, toana, ax, col, inds_v1, inds_lm, inds_pooled, top, top_sd, x_new):
Copy link
Collaborator

Choose a reason for hiding this comment

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

you should document what functions do

@@ -975,6 +975,14 @@ def get_extended_stimulus_presentations_for_session(session):
return extended_stimulus_presentations


def get_behavior_model_summary_table():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you use the behavior model results? If not, I'd remove this bit of code, since its just clutter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i dont think so. should be fine to remove it, unless marina or others use it.

from statsmodels.formula.api import ols
from statsmodels.stats.multicomp import (pairwise_tukeyhsd, MultiComparison)

def anova_tukey(svm_df, values_stat, label_stat='experience_levels', cres=['Slc17a7', 'Sst', 'Vip'], exp_level_all=['Familiar', 'Novel 1', 'Novel >1']):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of using multiple # to make the top comment, use <stuff>, that way it gets parsed by automated documentation viewers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure i'm following you here. Are you saying that instead of using # to make comments, I should use something else? I sometimes use """ or ''' .... but seems like you wrote something else? Can you clarify?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Its better to use:

def example():
   ` ` `
        This is a multiline
        comment
    ` ` `

Than

def example():
   # This is a multiline
   # comment

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see my original comment is confusing because github's markdown rendered my ``` as a code block.

# Convert a time window (relative to trial onset) to frames units, relative to "trace" begining; i.e. index on the trace whose time 0 is trace[samps_bef].
# samps_bef: number of frames before image/omission

import numpy as np
Copy link
Collaborator

Choose a reason for hiding this comment

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

Generally bad to include import statements within a function. Especially bad since numpy here is also imported in the entire module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why should they not be included within a function? do you mean they should be written outside the function, before it starts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Lots of reasons to import outside of functions. And yes, I mean all the import statements should be at the top of the module.

  • If you have import numpy as np inside every function, then it gets imported over and over again, and is simply slow
  • Its harder to update or debug the code if every function is importing different modules, which makes the code less transparent.
  • The python documentation says its best practice to have all import statements at the top of the module: https://docs.python.org/3/tutorial/modules.html

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