Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add support for jupyter notebook #414
Add support for jupyter notebook #414
Changes from all commits
db92ddf
97a0f5e
8ae4618
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@Fernando-crz @mateuslatrova help me sanity check my understanding of the
files
andcandidates
objects, files will be parsed and processed to get the list of imported packages, but this list will be contaminated by stdlib packages and local imports which will be removed in other processing steps (lines 165 and 171).The candidates list is simply a list of local files that could be imported (like an utils.py file) but shouldnt be a part of the requirements file. Is that it? If so, isnt candidates an awful name? what are they candidates of? also, this would explain why we are adding dir by dir to the list, as I raised in issue #424 and on our last meeting.
Maybe we could take this opportunity to rename those objects to something more intuitive? like local_files or local_modules since dirs can also be imported.
If you think this should be in another pr, ok
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.
maybe
py_files -> local_files
candidates -> local_modules
(pretty sure thts the correct nomenclature but we could use a double check)
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.
Can we just leave it for another PR? 🫥
We are not completely sure what is going on with those variables. We just copied the logic as it was before.
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.
@Fernando-crz my dude, you gotta understand whats going on and not just blindly reuse code haha
i opened issue #427 to keep track of this