Import tracking #66

jkanem · 2022-01-31T23:03:36Z

The serialization of the workflow, which results in dispatch_source.py, now includes imports from files external to the Covalent code-base.

I have added the tests to cover my changes.
I have updated the documentation, VERSION, and CHANGELOG accordingly.
I have read the CONTRIBUTING document.

statements that were used in end-user scripts. Helper functions _get_cova_imports, _get_imports_from_source, _scan_source_files, and parse_source_for_imports were added to _shared_files/utils.py to help get external imports. _write_dispatch_to_python_file now inccludes external import statements that were used in end-user scripts. A helper function, _remove_import_dupes, was written to remove duplicate entries from the result of _write_dispatch_to_python_file Added function get_imports to executors/base.py which is just a call to imports_from_sources in utils.py.

being included in serialization strings.

(dispatch_source.py) created in _write_dispatch_to_python_file.

CLAassistant · 2022-01-31T23:03:43Z

All committers have signed the CLA.

codecov · 2022-02-01T00:27:37Z

Codecov Report

Merging #66 (5e0a755) into develop (fb7f432) will increase coverage by 0.95%.
The diff coverage is 86.36%.

❗ Current head 5e0a755 differs from pull request most recent head 1f9396a. Consider uploading reports for the commit 1f9396a to get more accurate results

@@             Coverage Diff             @@
##           develop      #66      +/-   ##
===========================================
+ Coverage    71.50%   72.46%   +0.95%     
===========================================
  Files           27       27              
  Lines         1509     1565      +56     
===========================================
+ Hits          1079     1134      +55     
- Misses         430      431       +1

Impacted Files	Coverage Δ
covalent/_shared_files/defaults.py	`100.00% <ø> (ø)`
covalent/executor/base.py	`47.93% <60.00%> (+8.61%)`	⬆️
covalent/_results_manager/result.py	`90.13% <64.28%> (-3.77%)`	⬇️
covalent/_shared_files/utils.py	`85.50% <93.93%> (+6.50%)`	⬆️
covalent/_shared_files/config.py	`87.03% <0.00%> (-6.72%)`	⬇️
covalent/_workflow/electron.py	`77.51% <0.00%> (ø)`
covalent_dispatcher/_cli/service.py	`37.64% <0.00%> (+0.36%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb7f432...1f9396a. Read the comment docs.

wjcunningham7

I'm getting a problem in a very simple example, written in the Python console, which is not inspectable:

import covalent as ct
from covalent._shared_files.utils import get_serialized_function_str
@ct.electron
def test_func(input1: str, input2: int) -> str:
    return f"{input1} {input2}"
funcstr = get_serialized_function_str(test_func)
print(funcstr)

This says the source is not inspectable.

covalent/_results_manager/result.py

wjcunningham7 · 2022-02-01T04:12:38Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Jalani Kanem seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@jkanem make sure all your commits use the email associated with your GitHub account. You can do this with git commit --amend --author="My Name <my@email.com>" followed by a force push to your branch.

_write_dispatch_to_python_file.

jkanem · 2022-02-01T15:38:47Z

I'm getting a problem in a very simple example, written in the Python console, which is not inspectable:
import covalent as ct
from covalent._shared_files.utils import get_serialized_function_str
@ct.electron
def test_func(input1: str, input2: int) -> str:
    return f"{input1} {input2}"
funcstr = get_serialized_function_str(test_func)
print(funcstr)
This says the source is not inspectable.

The main work-horse of this functionality is the inspect module. Which works with files. Trying to use it in a console ~~or Jupyter~~ results in OSError: could not get source code.

kessler-frost

For the following workflow ran on a jupyter notebook,

import covalent as ct

@ct.electron
def sample_task(x):
    """Sample task."""

    return x ** 2

@ct.lattice
def sample_workflow(a, b):
    """Sample workflow."""

    c = sample_task(a)
    d = sample_task(b)
    return c, d

Sadly the dispatch_source file that I'm getting contains a huge list of imports. Maybe there is an already open issue for that or maybe we want to solve it in this one, I'm not sure.

covalent/_shared_files/utils.py

covalent/executor/base.py

covalent/_results_manager/result.py

Fixed typo in docstring in executor/base.py (get_imports).

…_function_str).

jkanem · 2022-02-02T01:39:00Z

For the following workflow ran on a jupyter notebook,
import covalent as ct

@ct.electron
def sample_task(x):
    """Sample task."""

    return x ** 2

@ct.lattice
def sample_workflow(a, b):
    """Sample workflow."""

    c = sample_task(a)
    d = sample_task(b)
    return c, d
Sadly the dispatch_source file that I'm getting contains a huge list of imports. Maybe there is an already open issue for that or maybe we want to solve it in this one, I'm not sure.

This is a Jupyter issue; Jupyter is external to the covalent code-base, and is seen as part of the end-user scripts. Possibly, making this work with Jupyter should be a separate issue.

kessler-frost · 2022-02-02T01:44:08Z

I agree. Just curious, were you able to reproduce this too?

kessler-frost · 2022-02-02T01:47:03Z

@jkanem Approved! Looks great, I'll merge this to develop once the tests pass and its synced with develop.

We'll need to fix the slowdown caused by get_serialized_str before this gets merged as it will make things worse.

flag whose default is False.

import statements in the serialized string.

now determined by a configuration parameter. Added some if/then clauses in the import scanning in order to avoid errors.

… that determines whether imports are scanned in get_serialized_function_str.

jkanem · 2022-02-04T00:13:35Z

I think this is ready for another look. The scanning for imports is now behind a configuration parameter in the sdk section:

[sdk]
full_dispatch_source = "false"

I'm open to a different name or section.
As mentioned, this will cause a regression in that, by default, covalent-related decorators will not be commented out in dispatch_source.py.

jkanem · 2022-02-05T00:47:20Z

This shouldn't be merged until #92 is.

kessler-frost · 2022-02-09T01:17:30Z

@jkanem Do you think this PR is also blocked by #80 ?

jkanem · 2022-02-09T01:28:36Z

@jkanem Do you think this PR is also blocked by #80 ?

No, I think this is a (non-ideal) solution to #80. This gets rid of the unreasonable time that it takes to get function strings, but it also gets rid of the import statements in dispatch_source.py.

jkanem · 2022-02-17T02:03:31Z

This PR is now deprecated by #123

jkanem added 4 commits January 31, 2022 20:09

Updated serialization_test.py to handle the import statements now

1860fe7

being included in serialization strings.

Updated VERSION and CHANGELOG.

70a59f7

Fixed some logic in removing duplicate import statements from the file

89e5488

(dispatch_source.py) created in _write_dispatch_to_python_file.

Modified serialization_test.py due to changes in pytest code.

8f436ff

jkanem requested review from wjcunningham7 and kessler-frost February 1, 2022 00:40

wjcunningham7 reviewed Feb 1, 2022

View reviewed changes

covalent/_results_manager/result.py Outdated Show resolved Hide resolved

String change in meassage written to dispatch_source.py in call to

8eb205b

_write_dispatch_to_python_file.

kessler-frost reviewed Feb 1, 2022

View reviewed changes

covalent/_shared_files/utils.py Outdated Show resolved Hide resolved

covalent/_shared_files/utils.py Outdated Show resolved Hide resolved

covalent/_shared_files/utils.py Outdated Show resolved Hide resolved

covalent/executor/base.py Outdated Show resolved Hide resolved

kessler-frost reviewed Feb 1, 2022

View reviewed changes

covalent/_results_manager/result.py Show resolved Hide resolved

jkanem added 3 commits February 1, 2022 22:08

Simplified code in _shared_files/utils.py (in parse_source_for_imports).

c65c4c2

Fixed typo in docstring in executor/base.py (get_imports).

Edits for more compact code in _shared_files/utils.py (get_serialized…

0074eb0

…_function_str).

Merge branch 'develop' into import_tracking

5d7d2cf

kessler-frost previously approved these changes Feb 2, 2022

View reviewed changes

kessler-frost mentioned this pull request Feb 2, 2022

Unreasonable amount of time spent in getting function string #80

Closed

jkanem added 5 commits February 3, 2022 16:00

The scanning of source files for imports is now optional with a

4779998

flag whose default is False.

Serialization tests now include tests for with and without including

fa9dd1d

import statements in the serialized string.

Whether to scan for imports in get_serialized_function_str is

83438f2

now determined by a configuration parameter. Added some if/then clauses in the import scanning in order to avoid errors.

Updated serialization tests to take advantage of the config parameter…

bb2c782

… that determines whether imports are scanned in get_serialized_function_str.

Merge branch 'develop' into import_tracking

5e0a755

jkanem linked an issue Feb 5, 2022 that may be closed by this pull request

Missing configuration parameters raise error #91

Closed

Merge branch 'develop' into import_tracking

1f9396a

wjcunningham7 assigned CLAassistant and jkanem and unassigned CLAassistant Feb 14, 2022

wjcunningham7 added covalent-dispatcher Related to the dispatcher module feature New feature addition labels Feb 14, 2022

jkanem closed this Feb 17, 2022

FyzHsn deleted the import_tracking branch February 14, 2023 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import tracking #66

Import tracking #66

jkanem commented Jan 31, 2022

CLAassistant commented Jan 31, 2022 •

edited

Loading

codecov bot commented Feb 1, 2022 •

edited

Loading

wjcunningham7 left a comment

wjcunningham7 commented Feb 1, 2022

jkanem commented Feb 1, 2022 •

edited

Loading

kessler-frost left a comment

jkanem commented Feb 2, 2022 •

edited

Loading

kessler-frost commented Feb 2, 2022

kessler-frost commented Feb 2, 2022

jkanem commented Feb 4, 2022

jkanem commented Feb 5, 2022

kessler-frost commented Feb 9, 2022

jkanem commented Feb 9, 2022

jkanem commented Feb 17, 2022

Import tracking #66

Import tracking #66

Conversation

jkanem commented Jan 31, 2022

CLAassistant commented Jan 31, 2022 • edited Loading

codecov bot commented Feb 1, 2022 • edited Loading

Codecov Report

wjcunningham7 left a comment

Choose a reason for hiding this comment

wjcunningham7 commented Feb 1, 2022

jkanem commented Feb 1, 2022 • edited Loading

kessler-frost left a comment

Choose a reason for hiding this comment

jkanem commented Feb 2, 2022 • edited Loading

kessler-frost commented Feb 2, 2022

kessler-frost commented Feb 2, 2022

jkanem commented Feb 4, 2022

jkanem commented Feb 5, 2022

kessler-frost commented Feb 9, 2022

jkanem commented Feb 9, 2022

jkanem commented Feb 17, 2022

CLAassistant commented Jan 31, 2022 •

edited

Loading

codecov bot commented Feb 1, 2022 •

edited

Loading

jkanem commented Feb 1, 2022 •

edited

Loading

jkanem commented Feb 2, 2022 •

edited

Loading