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

PR for whole code first CR #1

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

PR for whole code first CR #1

wants to merge 92 commits into from

Conversation

EfiHerbst31
Copy link
Owner

No description provided.

istarmap.py Outdated
@@ -0,0 +1,24 @@
import multiprocessing.pool as mp
Copy link
Owner Author

Choose a reason for hiding this comment

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

Add module level documentation for this file.

"""Summary line

More details...
"""

istarmap.py Outdated
import multiprocessing.pool as mp


def istarmap(self, func, iterable, chunksize=1):
Copy link
Owner Author

Choose a reason for hiding this comment

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

prefer creating a class that extends mp.Pool and adds this method. in your code you'll be using this class instead of mp.Pool.

istarmap.py Outdated
import multiprocessing.pool as mp


def istarmap(self, func, iterable, chunksize=1):
Copy link
Owner Author

Choose a reason for hiding this comment

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

Add pytype hints

istarmap.py Outdated


def istarmap(self, func, iterable, chunksize=1):
"""starmap-version of imap
Copy link
Owner Author

Choose a reason for hiding this comment

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

Summary line. Starts with capital, end with period.

main.py Outdated

FLAGS = flags.FLAGS

flags.DEFINE_integer('cpus', None, 'Number of CPUs to use in parallel. \
Copy link
Owner Author

Choose a reason for hiding this comment

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

for readability prefer breaking the string without slash:

flags.DEFINE_integer(
    'cpus', None, 
    ('Number of CPUs to use in parallel. default: all available cpus are used '
     'bla blab bla.'))

main.py Outdated
logging.debug('MicroRNA list: %s', miR_list)
if FLAGS.species == 'homo_sapiens' and 'hsa' not in miR_list[0]:
logging.exception('species is \'homo_sapiens\' but some of the microRNAs in the list do not belong to humans')
raise Exception('species is \'homo_sapiens\' but some of the microRNAs in the list do not belong to humans')
Copy link
Owner Author

Choose a reason for hiding this comment

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

Try using a less broad exception (maybe ValueError, or define your own exceptions as needed).

utils.py Outdated
import istarmap


warnings.simplefilter(action='ignore', category=FutureWarning)
Copy link
Owner Author

Choose a reason for hiding this comment

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

should be part of main? not sure anyone that imports this module would be interested in this filter

utils.py Outdated
the detected file name.

Raises:
Exception if more than one file was detected.
Copy link
Owner Author

Choose a reason for hiding this comment

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

use (existing or custom) less broad exceptions.
if only one type is raised, keep it in a single declaration: BlaBlaError if this happened or if that happened.

utils.py Outdated
pkl_file = glob.glob(data_path + '/*.pkl')
txt_files = glob.glob(data_path + '/*.txt')
tsv_files = glob.glob(data_path + '/*.tsv')
if pkl_file:
Copy link
Owner Author

Choose a reason for hiding this comment

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

this code repeats and can be extracted into a method. something like:

def get_data_file(data_path:str, file_extension:str) -> Optional[str]:
  """..."""
  files = glob.glob(os.path.join(data_path, '*.%s' extension))
  if len(files) > 1:
    raise ...
  return next(iter(files), None)

and then you can do:

data_file = (
   get_data_file(data_path, 'pkl') or
   get_data_file(data_path, 'txt') or
   get_data_file(data_path, 'tsv')
)
if not data_file:
  raise BlaError(...)

return data_file

utils.py Outdated
logging.error('Type of species not recognized, either "homo_sapiens" or \
"mus_musculus" are supported. For other species type, \
please download the mti data file and update mti_loader function')
raise Exception('Type of species not recognized, either "homo_sapiens" or \
Copy link
Owner Author

Choose a reason for hiding this comment

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

replace all the \ line breakers with breaking strings into lines

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.

None yet

2 participants