-
Notifications
You must be signed in to change notification settings - Fork 907
Install data and dependencies #403
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #403 +/- ##
==========================================
+ Coverage 96.07% 96.11% +0.04%
==========================================
Files 92 92
Lines 8023 8162 +139
==========================================
+ Hits 7708 7845 +137
- Misses 315 317 +2
Continue to review full report at Codecov.
|
kmax12
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Should be ready to merge after my comments are addressed
| directory = os.path.join(tmp_dir, os.path.dirname(members[0].path)) | ||
| for member in members: | ||
| if os.path.isfile(os.path.join(tmp_dir, member.path)): | ||
| directory = os.path.join(tmp_dir, os.path.dirname(member.path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should there be a break out of this loop after the directory is found?
featuretools/primitives/install.py
Outdated
| if prompt: | ||
| while True: | ||
| resp = input("Install primitives: %s? (Y/n) " % primitives_list) | ||
| if resp == "Y": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we update this to make it not case sensitive?
| files_to_copy.append(filepath) | ||
| all_primitives.add(primitive_obj.name) | ||
|
|
||
| if all_primitives != set(info['primitives']): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just need to add a test for this case
| for filepath in files: | ||
| primitive_name, primitive_obj = load_primitive_from_file(filepath) | ||
| if primitive_obj.name not in info['primitives']: | ||
| raise RuntimeError("Primitive %s not listed in " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just need to add a test for this case
kmax12
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
In addition to primitives files,
featuretools installwill now look for a folder named "data" and copy it tofeaturetools/primitives/data/. It will also look for "requirements.txt" and runpip install -r requirements.txtif it finds it.