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.

Sign up for GitHub

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

Jump to bottom

Add sys_platform environment marker to version checker #5437

Merged
merged 17 commits into from
May 6, 2021
Merged

Add sys_platform environment marker to version checker #5437

merged 17 commits into from
May 6, 2021

Conversation

achetver
Copy link
Contributor

@achetver achetver commented Apr 27, 2021 •
edited
Loading

Details:

Tickets:

  • 54366

Root cause analysis: Now version checker doesn't supports sys_platform environment marker.

Solution: Add sys_platform environment marker parsing and checking to parse_and_filter_versions_list().
Also correct parsing for lines with comments was added.

Code:

  • Comments
  • Code style (PEP8)
  • Transformation generates reshape-able IR
  • Transformation preserves original framework node names

Validation:

  • Unit tests;
  • Framework operation tests: N/A - No new enabled operations;
  • Transformation tests: N/A - No new implemented transformations;
  • Model Optimizer IR Reader check: N/A.

Documentation:

  • Supported frameworks operations list: N/A - No new supported operations;
  • Supported public models list: N/A - Nothing to add;
  • User guide update: N/A - Nothing to update.

@achetver achetver added the category: MO Model Optimizer label Apr 27, 2021
@achetver achetver added this to the 2021.4 milestone Apr 27, 2021
@achetver achetver requested review from a team, pavel-esir, mvafin, rkazants and yekruglov and removed request for a team and pavel-esir April 28, 2021 07:01
@achetver achetver marked this pull request as ready for review April 28, 2021 07:01
@achetver achetver requested a review from a team April 28, 2021 07:01
@achetver
Copy link
Contributor Author

@mvafin @rkazants @yekruglov Please, take a look at this changes.

@achetver
Copy link
Contributor Author

@mvafin @rkazants @yekruglov Please, take a look at this changes.

@achetver
Copy link
Contributor Author

@rkazants @mvafin @lazarevevgeny Please, review this PR.

@achetver achetver requested a review from rkazants April 29, 2021 13:47
@achetver
Copy link
Contributor Author

@rkazants @mvafin @lazarevevgeny Please, review this PR.

@achetver achetver requested a review from rkazants May 5, 2021 11:18
lazarevevgeny
lazarevevgeny suggested changes May 6, 2021
View reviewed changes
model-optimizer/mo/utils/versions_checker.py Outdated Show resolved Hide resolved
model-optimizer/mo/utils/versions_checker.py Outdated Show resolved Hide resolved
model-optimizer/mo/utils/versions_checker.py Outdated Show resolved Hide resolved
model-optimizer/mo/utils/versions_checker.py Outdated Show resolved Hide resolved
model-optimizer/mo/utils/versions_checker.py Outdated Show resolved Hide resolved
model-optimizer/unit_tests/mo/utils/versions_checker_test.py Outdated Show resolved Hide resolved
model-optimizer/mo/utils/versions_checker.py Outdated Show resolved Hide resolved
@achetver achetver requested a review from lazarevevgeny May 6, 2021 09:26
@achetver
Copy link
Contributor Author

achetver commented May 6, 2021

@lazarevevgeny Please, check last changes in this PR

@lazarevevgeny lazarevevgeny merged commit b3c1dd9 into openvinotoolkit:master May 6, 2021
@lazarevevgeny lazarevevgeny mentioned this pull request May 6, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yekruglov yekruglov yekruglov approved these changes

@rkazants rkazants rkazants approved these changes

@lazarevevgeny lazarevevgeny lazarevevgeny approved these changes

@mvafin mvafin mvafin approved these changes

Assignees

@lazarevevgeny lazarevevgeny

Labels
category: MO Model Optimizer
Projects
None yet
Milestone
2021.4
Development

Successfully merging this pull request may close these issues.
5 participants
@achetver @mvafin @lazarevevgeny @rkazants @yekruglov