provisional function on imports #89

Felice644 · 2022-05-09T18:25:29Z

-create the function that searches for imports from the notebook tree
-create the function that searches for reqirements from requiremets.txt
-create the function that searches for standard libraries from the python core

…tree -create the function that searches for reqirements from requiremets.txt -create the function that searches for standard libraries from the python core

louieQ · 2022-05-10T09:32:11Z

pynblint/notebook.py

+        #  non posso accedere ai set creati nella classe Repository,
+        #  come dovrei procedere?
+        # self.missing_requiremet = self.imported_package.difference(Repository.)


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.

louieQ · 2022-05-10T09:32:31Z

pynblint/notebook.py

@@ -19,7 +19,9 @@ class Notebook(RichRenderable):
    """

    def __init__(self, path: Path):
+        self.imported_package = None


Suggested change

self.imported_package = None

self.imported_packages = None

louieQ · 2022-05-10T09:35:07Z

pynblint/notebook.py

        self.path: Path = path
+        self.missing_requiremet: set


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.

louieQ · 2022-05-10T09:40:36Z

pynblint/repository.py

+        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)


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.

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)

louieQ · 2022-05-10T09:43:31Z

pynblint/repository.py

+        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"):


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.

louieQ · 2022-05-10T09:46:47Z

pynblint/repository.py

+                            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)


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

… the excessive recursion created earlier.

louieQ · 2022-05-14T09:49:57Z

pynblint/repository.py

-                    continue
-                elif f.endswith("Pipfile"):
-                    continue
+        path = os.path.dirname(os.path.abspath(__file__))


We don't need to dynamically fetch this information: the path of a repository is already present as a property in the class Repository. Use self.path to get it.

louieQ · 2022-05-14T09:51:31Z

pynblint/repository.py

+        # siccome il file di requirement si toverà nella root di
+        # progetto ho creato un file fittizio con le stesse
+        # informazioni per non spostare quello pre-esistente
+        file = "kk.txt"


I don't understand this.

the requirement.txt file that is present in the repository is located inside the "docs" folder and not in the project root where it should actually be. I wrote this passage in anticipation of what I believe is the future location of "requirement.txt". If, on the other hand, it should remain there where it is, I can change the route without problems.

louieQ · 2022-05-14T09:55:11Z

pynblint/repository.py

+                item.strip()
+                dependencies.add(tuple(item.split("==")))


Saving dependencies as tuples is unnecessary. You can save them as simple strings, stripping the version info (which we cannot deduce from the notebook).

So the certifi==2021.10.8 entry in requirements.txt would be stored as certifi in Pynblint.

I have inserted the step of the split to make it easier to check with the versions of the libraries to look for. Now I fix this step.

louieQ · 2022-05-14T09:56:56Z

pynblint/repository.py

-                            coredependecies.add(f)
-                break
+        modules = sys.modules
+        pattern: Pattern[str] = re.compile(r".*\\\\Python\\\\Python37\\\\lib\\\\.*.py")


This regex is tied to your installation of Python. It would not work with a different Python version (e.g., Python 3.8) or a different virtual environment name.

At least, I would generalize the Python version.

Of course, it was an oversight. I correct it immediately.

louieQ · 2022-05-14T09:58:12Z

pynblint/repository.py

+        for i in modules.items():
+            # print(" PPOSIZIONE 0:" + i[0])
+            # print(" PPOSIZIONE 1:" + str(i))
+            if pattern.match(str(i)):
+                coredependecies.add(i[0])
+        print(coredependecies)


I guess these are debug print statements that will be removed.

louieQ · 2022-05-14T10:15:52Z

pynblint/repository.py

-                        if f.endswith(".py"):
-                            coredependecies.add(f)
-                break
+        modules = sys.modules


Are you sure that this solution is appropriate? From this SO thread I learn that:

sys.modules - only shows modules that have already been loaded

The solution proposed in one of the answers to the same SO question seems to be a lot cleaner to me and might be the way to go: i.e., using isort's place_module function or getting the list of core modules like this:

from isort.stdlibs.py39 import stdlib for name in sorted(stdlib): print(name)

- Added the test class and related tests for the functions present in the repository.

- Refactoring of code in repository

…modules/packeges_imported # Conflicts: # pynblint/notebook.py

- Minor changes in the repository class.

The parser funcion for `environment.yml` files is ready and tested.

Add further test cases for `Notebook._get_imported_packages`

Updates and fixes the functions to detect undeclared dependencies

codecov-commenter · 2022-05-31T08:37:06Z

Codecov Report

Merging #89 (f24380f) into master (8c2b091) will increase coverage by 5.18%.
The diff coverage is 81.32%.

@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
+ Coverage   56.84%   62.03%   +5.18%     
==========================================
  Files          18       18              
  Lines         913     1151     +238     
==========================================
+ Hits          519      714     +195     
- Misses        394      437      +43

Flag	Coverage Δ
pytest	`62.03% <81.32%> (+5.18%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pynblint/nb_linting.py	`64.65% <11.53%> (-15.35%)`	⬇️
pynblint/exceptions.py	`50.00% <50.00%> (+50.00%)`	⬆️
pynblint/core_models.py	`71.59% <79.16%> (+3.94%)`	⬆️
tests/unit/test_core_models.py	`99.22% <100.00%> (+5.89%)`	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

…cies.

-create the function that searches for imports from the notebook ast …

8b8446a

…tree -create the function that searches for reqirements from requiremets.txt -create the function that searches for standard libraries from the python core

Felice644 requested a review from louieQ May 9, 2022 18:25

louieQ reviewed May 10, 2022

View reviewed changes

-improved the find functions of the python core libraries and reduced…

5930e7b

… the excessive recursion created earlier.

Felice644 requested a review from louieQ May 11, 2022 18:31

louieQ requested changes May 14, 2022

View reviewed changes

Felice644 requested a review from louieQ May 14, 2022 15:03

Felice644 and others added 6 commits May 19, 2022 15:25

- Fixed the function for parsing of .txt, .toml, .yaml file

527c709

- Added the test class and related tests for the functions present in the repository.

- just a little fix on the name

a501026

- First attempt to correct the error on the tests

c928c12

- Fixed error on the tests

88b1a91

- Fixed previous comments in notebook class

c638c58

Minor improvements in notebook.py

d6beae7

louieQ added the linting rule A new check performed by pynblint label May 22, 2022

Felice644 and others added 16 commits May 24, 2022 15:09

- Added test and test class for notebook functions.

9cbed73

- Refactoring of code in repository

Merge remote-tracking branch 'origin/modules/packeges_imported' into …

14f9c86

…modules/packeges_imported # Conflicts: # pynblint/notebook.py

- Update project to the last versions

39ff970

- Fixed failing repository tests.

4a94a5f

- Minor changes in the repository class.

Integrate the latest refactoring (core_models.py)

6163d3b

Improve the parsing strategy for setup.py files

69cfee8

Repo requirements parsing fixtures

c7a9359

Refactor requirement file parsers

3c68e02

The parser funcion for `environment.yml` files is ready and tested.

Perser for requirements.txt ready and tested

e3f90a8

Parser for pyproject.toml ready and tested

4823f1f

Parser for setup.py ready and tested

c623078

Parser for Pipfile ready and tested

28093f3

Handle exceptions while parsing requirement files

2138192

Improve the management of magic functions

de8a6a1

Add further test cases for `Notebook._get_imported_packages`

Check the presence of undeclared dependencies

03f7185

Merge pull request #93 from collab-uniba/merge_PR_89

f24380f

Updates and fixes the functions to detect undeclared dependencies

Felice644 and others added 3 commits June 1, 2022 18:42

- Added tests for dependencies_unmanaged and test_undeclared_dependen…

4808cd3

…cies.

- Partial solution for the test case.

3c569d2

Fixed test_undeclared_dependencies

2f61f4a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provisional function on imports #89

provisional function on imports #89

Felice644 commented May 9, 2022

louieQ May 10, 2022

louieQ May 10, 2022

louieQ May 10, 2022

louieQ May 10, 2022

louieQ May 10, 2022

louieQ May 10, 2022

louieQ May 14, 2022

louieQ May 14, 2022

Felice644 May 14, 2022

louieQ May 14, 2022

Felice644 May 14, 2022

louieQ May 14, 2022

Felice644 May 14, 2022

louieQ May 14, 2022

louieQ May 14, 2022

codecov-commenter commented May 31, 2022 •

edited

Loading

provisional function on imports #89

Are you sure you want to change the base?

provisional function on imports #89

Conversation

Felice644 commented May 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented May 31, 2022 • edited Loading

Codecov Report

codecov-commenter commented May 31, 2022 •

edited

Loading