0
\$\begingroup\$

Details about code:

  • Dataframe (x) shape: (20494463, 31)
  • In this Dataframe, each id shows up multiple times with different events and the event dates. What I'm looking for is the time between the first time event A occurs and the first time event B occurs for each id
  • feature_final.index.values is another Dataframe whose indexes are the id's I need to work with

def feature(x):
   times = []
   for j in feature_final.index.values:

     pat = x[x.id == j]
     min_dates_j = pat.groupby(["event_description"]).min()

     try:
         time = ((min_dates_j.loc["A","event_description"] -  min_dates_j.loc["B","event_description"]).days) 
     except:
         #RX or DX not present
         time = np.NaN()

     times.append(time)

     return times
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Are you sure that the code is correctly posted? The times result will never contain more than one value. \$\endgroup\$ – 200_success Oct 30 '17 at 3:05
  • \$\begingroup\$ How about adding some sample data? It will make writing some feedback much easier. \$\endgroup\$ – miradulo Nov 6 '17 at 19:14
1
\$\begingroup\$

Some general suggestions:

  • Run the code through pycodestyle to get several suggestions for writing idiomatic Python:
    • indentation is not a multiple of four

    • missing whitespace after ','

    • multiple spaces after operator

    • line too long

    • do not use bare except' [sic]

    • block comment should start with '# '

  • Don't use single letter variables - it makes it very hard to maintain the code.
  • Don't shadow common names like time - if you ever need the time module in the code you'll have to rename the import or your variable.
  • You have unnecessary outer parens in the time = … line.
  • Indexing by a non-trivial expression is confusing. Assign x.id == j to a variable first so the maintainer knows what it means.
  • feature_final is accessed like a global. You should instead pass it in.
  • If you're processing a huge list you might want to try returning an iterable instead of a list - that way other code can start processing the output before it's complete.
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the feedback! Do you have any feedback on how to improve its performance? \$\endgroup\$ – madsthaks Oct 30 '17 at 1:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.