Skip to content
This repository has been archived by the owner on Jan 27, 2024. It is now read-only.

add --zip option to zip the submission file prior to uploading it #52

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions kaggle_cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@

from cliff.command import Command


CONFIG_DIR_NAME = '.kaggle-cli'
CONFIG_FILE_NAME = 'config'
DATA_OPTIONS = set(['username', 'password', 'competition'])
DATA_OPTIONS = set(['username', 'password', 'competition', 'zip'])


def get_config(config_path):
Expand Down Expand Up @@ -85,6 +84,7 @@ def get_parser(self, prog_name):
parser.add_argument('-u', '--username', help='username')
parser.add_argument('-p', '--password', help='password')
parser.add_argument('-c', '--competition', help='competition')
parser.add_argument('-z', '--zip', help='zip the submission file before uploading?', action='store_true')
parser.add_argument(
'-g',
'--global',
Expand All @@ -98,7 +98,7 @@ def take_action(self, parsed_args):
parsed_arg_dict = vars(parsed_args)

if DATA_OPTIONS & set(
filter(lambda x: parsed_arg_dict[x], parsed_arg_dict)
filter(lambda x: parsed_arg_dict[x], parsed_arg_dict)
Copy link
Owner

Choose a reason for hiding this comment

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

Is it an accident duplicated indent?

Copy link
Author

Choose a reason for hiding this comment

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

IDE issues I guess

):
if parsed_arg_dict['global']:
config_dir = os.path.join(
Expand Down Expand Up @@ -135,6 +135,11 @@ def take_action(self, parsed_args):
parsed_arg_dict['competition']
)

if parsed_arg_dict['zip']:
config.set('user', 'zip', 'yes')
else:
config.set('user', 'zip', 'no')

with open(config_path, 'w') as config_file:
config.write(config_file)
else:
Expand Down
26 changes: 20 additions & 6 deletions kaggle_cli/submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def get_parser(self, prog_name):
parser.add_argument('-u', '--username', help='username')
parser.add_argument('-p', '--password', help='password')
parser.add_argument('-z', '--zip', type=self._str2bool, nargs='?', const=True, default=False,
Copy link
Owner

Choose a reason for hiding this comment

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

Why don't you use the same argument setting as config.py?

i.e. parser.add_argument('-z', '--zip', help='zip the submission file before uploading?', action='store_true')

Copy link
Author

Choose a reason for hiding this comment

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

agree

help='zip the submission file before uploading')
help='whether to zip the submission file before uploading')

return parser

Expand All @@ -35,7 +35,12 @@ def take_action(self, parsed_args):
username = config.get('username', '')
password = config.get('password', '')
competition = config.get('competition', '')
zip = config.get('zip', False)
zip_flag = config.get('zip', 'no')
Copy link
Owner

Choose a reason for hiding this comment

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

You can use getboolean method instead.

Copy link
Author

Choose a reason for hiding this comment

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

@floydwch but the config object here is just a python dict, it's not a ConfigParser object

Copy link
Owner

Choose a reason for hiding this comment

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

This issue should be resolved at the config provider level, I'll address it.


if Submit._str2bool(zip_flag):
zip = True
else:
zip = False

browser = common.login(username, password)
base = 'https://www.kaggle.com'
Expand All @@ -46,7 +51,7 @@ def take_action(self, parsed_args):
entry = parsed_args.entry
message = parsed_args.message

archive_name = Submit._rand_str(10)+'.zip'
archive_name = Submit._rand_str(10) + '.zip'
Copy link
Owner

Choose a reason for hiding this comment

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

Since the uploaded file's name will also show on the Kaggle's dashboard. A related name might be better. i.e. append the uuid or timestamp to the original submission file's name.

And another concern there is, for myself habit, the submission files are already suffixed a timestamp to distinguish the trial. Adding additional suffix is duplicated.

Copy link
Author

@queirozfcom queirozfcom Oct 23, 2017

Choose a reason for hiding this comment

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

I agree. What if I prepend a short random string instead?


if zip:
with zipfile.ZipFile(archive_name, 'w', zipfile.ZIP_DEFLATED) as zf:
Expand Down Expand Up @@ -122,10 +127,11 @@ def take_action(self, parsed_args):
@staticmethod
Copy link
Owner

Choose a reason for hiding this comment

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

Static method is not the best choice in such case. And an underscore prefix method indicates this method is private, which means it'll access the object's private fields, but it's not the case here. I suggest you extract the method to the module scope, i.e. move the _make_archive_name outside the Submit class but still in the submit.py.

def _str2bool(v):
"""
parse boolean values
parse truthy/falsy strings into booleans

https://stackoverflow.com/a/43357954/436721
:return:
:param v: the string to be parsed
:return: a boolean value
"""
if v.lower() in ('yes', 'true', 't', 'y', '1'):
return True
Expand All @@ -136,4 +142,12 @@ def _str2bool(v):

@staticmethod
def _rand_str(length):
return uuid.uuid4().hex[:length-1]
"""
this is used to prevent caching issues

https://stackoverflow.com/a/34017605/436721

:param length: integer length
:return: a random string of the given length
"""
return uuid.uuid4().hex[:length - 1]