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

provisional function on imports #89

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8b8446a
-create the function that searches for imports from the notebook ast …
Felice644 May 9, 2022
5930e7b
-improved the find functions of the python core libraries and reduced…
Felice644 May 11, 2022
527c709
- Fixed the function for parsing of .txt, .toml, .yaml file
Felice644 May 19, 2022
a501026
- just a little fix on the name
Felice644 May 19, 2022
c928c12
- First attempt to correct the error on the tests
Felice644 May 20, 2022
88b1a91
- Fixed error on the tests
Felice644 May 21, 2022
c638c58
- Fixed previous comments in notebook class
Felice644 May 21, 2022
d6beae7
Minor improvements in `notebook.py`
louieQ May 22, 2022
9cbed73
- Added test and test class for notebook functions.
Felice644 May 24, 2022
14f9c86
Merge remote-tracking branch 'origin/modules/packeges_imported' into …
Felice644 May 24, 2022
39ff970
- Update project to the last versions
Felice644 May 24, 2022
4a94a5f
- Fixed failing repository tests.
Felice644 May 25, 2022
6163d3b
Integrate the latest refactoring (`core_models.py`)
louieQ May 27, 2022
69cfee8
Improve the parsing strategy for `setup.py` files
louieQ May 27, 2022
c7a9359
Repo requirements parsing fixtures
louieQ May 27, 2022
3c68e02
Refactor requirement file parsers
louieQ May 27, 2022
e3f90a8
Perser for `requirements.txt` ready and tested
louieQ May 27, 2022
4823f1f
Parser for `pyproject.toml` ready and tested
louieQ May 27, 2022
c623078
Parser for `setup.py` ready and tested
louieQ May 27, 2022
28093f3
Parser for `Pipfile` ready and tested
louieQ May 27, 2022
2138192
Handle exceptions while parsing requirement files
louieQ May 30, 2022
de8a6a1
Improve the management of magic functions
louieQ May 30, 2022
03f7185
Check the presence of undeclared dependencies
louieQ May 30, 2022
f24380f
Merge pull request #93 from collab-uniba/merge_PR_89
Felice644 May 31, 2022
4808cd3
- Added tests for dependencies_unmanaged and test_undeclared_dependen…
Felice644 Jun 1, 2022
3c569d2
- Partial solution for the test case.
Felice644 Jun 6, 2022
2f61f4a
Fixed `test_undeclared_dependencies`
louieQ Jun 10, 2022
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
2 changes: 1 addition & 1 deletion pynblint/nb_linting.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ def long_multiline_python_comment(notebook: Notebook) -> List[Cell]:
show_details=False,
),
LintDefinition(
slug="long_multiline_python_comment",
slug="long-multiline-python-comment",
description="One or more code cells in this notebook contain Python comments "
f"of {settings.max_multiline_python_comment} or more consecutive lines.",
recommendation="For improved notebook readability, prefer using Markdown "
Expand Down
20 changes: 20 additions & 0 deletions pynblint/notebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ class Notebook(RichRenderable):
"""

def __init__(self, path: Path):
self.imported_package = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
self.imported_package = None
self.imported_packages = None

self.path: Path = path
self.missing_requiremet: set
Copy link
Collaborator

Choose a reason for hiding this comment

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

This information is not supposed to be stored here. It will be computed as the result of a linting function dedicated to checking the presence of import statements that do not map to the declared project requirements.


# Read the notebook with nbformat
with open(self.path) as f:
Expand All @@ -44,6 +46,10 @@ def __init__(self, path: Path):
self.ast = ast.parse(self.script)
except SyntaxError:
self.has_invalid_python_syntax = True
self.imported_package = self._get_import_package_set()
# non posso accedere ai set creati nella classe Repository,
# come dovrei procedere?
# self.missing_requiremet = self.imported_package.difference(Repository.)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This check should be performed within a dedicated linting function (inside nb_linting.py, I guess).
Here we are just defining notebook properties; the notebook property that we need to add in order to make the check feasible is the set of imported packages, as you already do at line 49.


@property
def code_cells(self) -> List[Cell]:
Expand All @@ -63,6 +69,20 @@ def initial_cells(self) -> List[Cell]:
def final_cells(self) -> List[Cell]:
return self.cells[-settings.final_cells :] # noqa: E203

def _get_import_package_set(self):
import_set = set()
for node in ast.walk(self.ast):
if isinstance(node, ast.Import):
for name in node.names:
import_set.add(name.name.split(".")[0])
elif isinstance(node, ast.ImportFrom):
if node.level > 0:
# Relative imports always refer to the current package.
continue
# assert node.module
import_set.add(node.module.split(".")[0])
return import_set

def __len__(self) -> int:
return len(self.cells)

Expand Down
48 changes: 47 additions & 1 deletion pynblint/repository.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import sys
import tempfile
import zipfile
from abc import ABC
Expand All @@ -23,6 +24,8 @@ def __init__(self, path: Path):

# Extracted content
self.notebooks: List[Notebook] = [] # List of Notebook objects
self.dependencies_module: set = self._get_dependencies()
self.core_library: set = self._get_core_dependecies()

def retrieve_notebooks(self):

Expand Down Expand Up @@ -67,6 +70,50 @@ def large_file_paths(self) -> List[Path]:
large_files.append(file_path)
return large_files

def _get_dependencies(self) -> set:
dependencies = set()
for root, dirs, files in os.walk(self.path):
for f in files:
# check if exist a requiremets.txt file
if f.endswith("requirements.txt"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This recursive search is not necessary. requirements.txt – as well as other files in which dependency declaration typically occurs – is expected to be placed at the root of a Python project.

try:
with open((Path(root) / Path(f)), "r") as fi:
file_row = fi.read()
# print(file_row)
tmp = file_row.split("\n")
for item in tmp:
dependencies.add(item)
# dependencies.add(file_row)
# dependencies.add(file_row)
Copy link
Collaborator

Choose a reason for hiding this comment

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

When you parse a package from requirements.txt, make sure to strip the version constraint information (e.g., the minimum version number)

except Exception as e:
print(e)
elif f.endswith("poetry.lock"):
continue
elif f.endswith("pyproject.toml"):
continue
elif f.endswith("environment.yml"):
continue
elif f.endswith("setup.py"):
continue
elif f.endswith("Pipfile"):
continue
# print(dependencies)

return dependencies

def _get_core_dependecies(self) -> set:
coredependecies = set()
# path = None
for i in sys.path:
if i.endswith("\\lib\\site-packages"):
for root, dirs, files in os.walk(i):
for f in files:
if f.endswith(".py"):
coredependecies.add(f)
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. as you can read in this Stack Overflow post, site-packages does not contain just Python's core packages, but it's rather "the target directory of manually built Python packages" in general.
  2. Is a dynamic check like this really necessary? This seems like a fixed information to me (at least if we consider a single Python version at a time)

break

return coredependecies


class LocalRepository(Repository):
"""
Expand Down Expand Up @@ -114,7 +161,6 @@ class GitHubRepository(Repository):
"""

def __init__(self, github_url: str):

self.url = github_url

# Clone the repo in a temp directory
Expand Down