3
\$\begingroup\$

I have some side projects by using a private module I have. My main goals are to match the list if they started with the pre-defined prefix, then put them into a list. From that filtered list, I extract two elements each time and modify their label name. I am trying to clean up my code to make it more concise and user-friendly, also to make sure asking some questions before actually modify the file label. Ideally, I want to have a way to list down the file's label that is going to edit before actually editing them, if that is possible.

for k, v in sessList.items():
    # get the acquisitions in one session
    acqList = module.get_session_acqs(session_id = k, sort = {'timestamp': 'asc'} )
    conversion(acqList)

import itertools
import numpy as np
from pytz import utc
from datetime import *
from dateutil import *
from datetime import timedelta
from itertools import *
from math import *


def conversion(acqList):


  prefix = ['MCS','FT', 'FS', 'GNo', 'Rs', 'WHw', 'Cy']
  newList = []

#to check if there is any of the items contains the prefix
  for e in acqList:
    for pf in prefix:
      if(e.label.startswith(pf)):
        newList.append(e)

  data = np.arange(len(newList))

  def pairwise(iterable):
    return zip(*[iter(iterable)]*2)

  for v, w in pairwise(data):

    labelA = newList[v].label
    labelB = newList[w].label

#checking when the labelA or labelB has MCS then change the name
    if 'MCS' in labelA:
      newLabel = labelB + '_mc'

      acqID = newList[v].id
#here is when the modify happens from my module
      module.modify_acq(acquisition_id = acqID, label = newLabel )

    elif 'MCS' in labelB:
      newLabel = labelA + '_mc'

      acqID = newList[w].id
#here is when the modify happens from my module
      module.modify_acq(acquisition_id = acqID, label = newLabel )

\$\endgroup\$
8
  • 1
    \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How to Ask for examples, and revise the title accordingly. \$\endgroup\$ Mar 18 '20 at 9:41
  • \$\begingroup\$ @TobySpeight thanks for pointing it out. I will try to edit my title. \$\endgroup\$
    – xd3262nd
    Mar 18 '20 at 14:03
  • \$\begingroup\$ Ideally, I want to have a way to list down the file's label - If your code does not do this now, CR is not the place to help you; you're better off with Stack Overflow. Code Review can only help you with current, implemented, working code. \$\endgroup\$
    – Reinderien
    Mar 19 '20 at 16:39
  • \$\begingroup\$ Your code, as it stands, will not run; the indentation on your comments is incorrect. \$\endgroup\$
    – Reinderien
    Mar 19 '20 at 16:39
  • 1
    \$\begingroup\$ Bizarre. I'm downgrading from 'it's not syntactically valid' - because, bizarrely, it is - to "no one should ever do that". \$\endgroup\$
    – Reinderien
    Mar 22 '20 at 0:16
2
\$\begingroup\$

Indentation

This:

def conversion(acqList):


  prefix = ['MCS','FT', 'FS', 'GNo', 'Rs', 'WHw', 'Cy']
  newList = []

#to check if there is any of the items contains the prefix
  for e in acqList:

is (as a surprise to me) valid syntax, but that doesn't mean it's a good idea. The beginning of a comment, when that comment is the only thing on the line, should match the level of indentation of the block it's in. To do otherwise is pretty confusing.

You also seem to be using two-space indentation, when the standard is four.

If-parens

This:

  if(e.label.startswith(pf)):

should become

if e.label.startswith(pf):

Nested functions

There are sometimes good reasons for nested functions, but so far as I can see, this:

  def pairwise(iterable):
    return zip(*[iter(iterable)]*2)

doesn't have any of them. You're better off moving this function to global scope.

PEP8 spacing

By the PEP8 formatting standard, this:

  module.modify_acq(acquisition_id = acqID, label = newLabel )

would become

module.modify_acq(acquisition_id=acq_id, label=new_label)

Mutability

This:

  prefix = ['MCS','FT', 'FS', 'GNo', 'Rs', 'WHw', 'Cy']

never changes, so it should be made an immutable tuple:

  prefix = ('MCS','FT', 'FS', 'GNo', 'Rs', 'WHw', 'Cy')

Namespace pollution

These:

from datetime import *
from dateutil import *
from datetime import timedelta
from itertools import *
from math import *

have a few problems. First, your timedelta import is redundant, because you already import *.

Also, it's not a great idea to import everything from these modules. You're better off either

  1. importing specific symbols from these modules as necessary, or
  2. importing only the module itself (i.e. import math) and then using fully-qualified references.

Either of those will significantly clean up your namespace.

\$\endgroup\$

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.