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

grc: update id blackist with imports #4522

Merged
merged 1 commit into from Apr 15, 2021

Conversation

dl1ksv
Copy link
Contributor

@dl1ksv dl1ksv commented Apr 11, 2021

This is an alternative solution for #4480
It uses the import statements to update the blacklist for id's.

That is: If you don't

from gnuradio import audio

you can use audio as id. Otherwise it is blacklisted

Signed-off-by: Volker Schroer 3470424+dl1ksv@users.noreply.github.com

@dl1ksv dl1ksv mentioned this pull request Apr 11, 2021
@dl1ksv dl1ksv requested a review from mormj April 11, 2021 13:12
@willcode
Copy link
Member

I think we need a deterministic solution, even something as simple as requiring the id to start with a certain prefix. Better yet, a solution that allows the user to use whatever id they desire.

@dl1ksv
Copy link
Contributor Author

dl1ksv commented Apr 11, 2021

@willcode : I disagree. This solution is deterministic and works even if new modules are added. And if you ave to look for bugs the generated python code contains the id's the user has given his modules. If you modify the id's you can't overview the side effects. And you cannot allow the user to use id's whatever they desire as the may conflict with python rules.

@willcode
Copy link
Member

OK, it's better than using the blacklist in general. It could, however, surprise a user when an import gets added by a new block and an id is now invalid. Not a huge deal. Can this be done without using exec()?

@dl1ksv
Copy link
Contributor Author

dl1ksv commented Apr 11, 2021

I know no other way. This part of code is taken from Flowgraph.py

@willcode
Copy link
Member

Something like the following. This also gets rid of any performance issue with actually importing packages.

import ast

imports = """
import sys
import os, time
from gnuradio import blocks
from gnuradio import audio
from gnuradio import window, fft as some_other_name
"""

tree = ast.parse(imports)
for expr in tree.body:
    if type(expr) in (ast.Import, ast.ImportFrom):
        for name in expr.names:
            print(name.asname or name.name)

Copy link
Member

@skoslowski skoslowski left a comment

Choose a reason for hiding this comment

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

This is great! Thanks @dl1ksv for the implementation.

class ValidateError(Exception):
"""Raised by validate functions"""


@validates('id')
def validate_block_id(param):
def validate_block_id(param,bl):
Copy link
Member

Choose a reason for hiding this comment

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

little nit: bt is not a very (self-)descriptive name, why not use sth. like black_listed_ids.

pass
except Exception:
log.exception('Failed to evaluate import expression "{0}"'.format(expr), exc_info=True)
pass
Copy link
Member

Choose a reason for hiding this comment

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

This re-creates the black list for every block - even though it does not change. My suggestion here is to have the FlowGraph class build that list as it creates the namespace (and runs the imports anyway). Then have getter, get_imported_names() which you can use below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Many thanks for your hints. This makes the code smoother

Signed-off-by: Volker Schroer <3470424+dl1ksv@users.noreply.github.com>
@willcode willcode added Backport-3.8 Should be backported to 3.8 Backport-3.9 Should be backported to 3.9 labels Apr 12, 2021
@mormj mormj merged commit 035ed74 into gnuradio:master Apr 15, 2021
@willcode
Copy link
Member

willcode commented Apr 15, 2021

Would it be useful to add keyword.kwlist to the list?

Python 2

>>> keyword.kwlist
['and', 'as', 'assert', 'break', 'class', 'continue', 'def', 'del', 'elif', 'else', 'except', 'exec', 'finally', 'for', 'from', 'global', 'if', 'import', 'in', 'is', 'lambda', 'not', 'or', 'pass', 'print', 'raise', 'return', 'try', 'while', 'with', 'yield']

Python 3

>>> keyword.kwlist
['False', 'None', 'True', '__peg_parser__', 'and', 'as', 'assert', 'async', 'await', 'break', 'class', 'continue', 'def', 'del', 'elif', 'else', 'except', 'finally', 'for', 'from', 'global', 'if', 'import', 'in', 'is', 'lambda', 'nonlocal', 'not', 'or', 'pass', 'raise', 'return', 'try', 'while', 'with', 'yield']

@dl1ksv dl1ksv deleted the namespace_as_blacklist branch April 16, 2021 09:30
@willcode willcode added ported-to-3.9 Has been ported to 3.9 and removed Backport-3.9 Should be backported to 3.9 labels Apr 16, 2021
@willcode willcode added ported-to-3.8 Has been ported to 3.8 and removed Backport-3.8 Should be backported to 3.8 labels Apr 19, 2021
solomonbstoner added a commit to solomonbstoner/gnuradio that referenced this pull request May 9, 2021
I refer to the mailing list thread on May 8 2021 regarding "Embedded
Python Block Tutorial Param ID Error".

The bug is caused by gnuradio#4522 as names of embedded python blocks
such as `epy_block_0` are blacklisted because of "blocks" in the
argument `black_listed_ids`.

Signed-off-by: Solomon Tan <solomonbstoner@yahoo.com.au>
solomonbstoner added a commit to solomonbstoner/gnuradio that referenced this pull request May 9, 2021
I refer to the mailing list thread on May 8 2021 regarding "Embedded
Python Block Tutorial Param ID Error".

The bug is caused by gnuradio#4522 as names of embedded python blocks
such as `epy_block_0` are blacklisted because of "blocks" in the
argument `black_listed_ids`. It affects both embedded python blocks and
modules. This commit fixes the bug by granting exemption for these two
types of blocks from id validation.

Signed-off-by: Solomon Tan <solomonbstoner@yahoo.com.au>
solomonbstoner added a commit to solomonbstoner/gnuradio that referenced this pull request May 15, 2021
I refer to the mailing list thread on May 8 2021 regarding "Embedded
Python Block Tutorial Param ID Error".

The bug is caused by gnuradio#4522 as names of embedded python blocks
such as `epy_block_0` are blacklisted because of "blocks" in the
argument `black_listed_ids`. It affects both embedded python blocks and
modules. This commit fixes the bug by granting exemption for these two
types of blocks from id validation.

This commit makes the regex more strictly follow the naming convention
of epy blocks and modules so that only these will be fast-tracked
through the id validation. The naming convention appears to be a "epy"
prefix, followed by "block" or "module", and a number suffix.

This regex also takes into account the user copying-and-pasting the epy
blocks resulting in id names like `epy_block_0_0` and `epy_block_0_0_0`.

Instead of checking for epy blocks or modules using the naming
convention in the id like commit ac383d1 does, this commit checks by
looking at the block's key. All epy blocks and modules are identified
with the key `epy_block` and `epy_module` respectively.

This commit fixes the bug by changing the import name such that it does
not coincide with the generated id for epy blocks and modules. This
means that there no longer needs to be an exception case in the
validation id function for epy blocks and modules.

This commit fixes the bug by giving epy blocks and modules a new
attribute that grants them exemption from blacklist id validation.
Unlike v1, v2 and v3, this does not depend on the naming convention of
the block ids or block keys, which could change in the future, making
the code less of a hassle to maintain.

Improved on v5 (commit b85da80) with the following changes.
1. Changed `exempt_from_id_validation` from an instance variable to a
class variable since all instances of epy blocks and modules are
affected by the bug mentioned in v5.
2. Changed `hasattr` to `getattr` to prevent the bug where a block is
exempted from id validation when `exempt_from_id_validation=False`.

Improved from v6 (commit e66ed85). This grants the exemption to
blocks/modules only after they are marked for blacklist. This change
makes sure that the id exemption is used only to prevent the
blacklisting bug. The blocks/modules will still go through other forms
of id validation such as the naming convention check.

Signed-off-by: Solomon Tan <solomonbstoner@yahoo.com.au>
marcusmueller pushed a commit that referenced this pull request May 15, 2021
I refer to the mailing list thread on May 8 2021 regarding "Embedded
Python Block Tutorial Param ID Error".

The bug is caused by #4522 as names of embedded python blocks
such as `epy_block_0` are blacklisted because of "blocks" in the
argument `black_listed_ids`. It affects both embedded python blocks and
modules. This commit fixes the bug by granting exemption for these two
types of blocks from id validation.

This commit makes the regex more strictly follow the naming convention
of epy blocks and modules so that only these will be fast-tracked
through the id validation. The naming convention appears to be a "epy"
prefix, followed by "block" or "module", and a number suffix.

This regex also takes into account the user copying-and-pasting the epy
blocks resulting in id names like `epy_block_0_0` and `epy_block_0_0_0`.

Instead of checking for epy blocks or modules using the naming
convention in the id like commit ac383d1 does, this commit checks by
looking at the block's key. All epy blocks and modules are identified
with the key `epy_block` and `epy_module` respectively.

This commit fixes the bug by changing the import name such that it does
not coincide with the generated id for epy blocks and modules. This
means that there no longer needs to be an exception case in the
validation id function for epy blocks and modules.

This commit fixes the bug by giving epy blocks and modules a new
attribute that grants them exemption from blacklist id validation.
Unlike v1, v2 and v3, this does not depend on the naming convention of
the block ids or block keys, which could change in the future, making
the code less of a hassle to maintain.

Improved on v5 (commit b85da80) with the following changes.
1. Changed `exempt_from_id_validation` from an instance variable to a
class variable since all instances of epy blocks and modules are
affected by the bug mentioned in v5.
2. Changed `hasattr` to `getattr` to prevent the bug where a block is
exempted from id validation when `exempt_from_id_validation=False`.

Improved from v6 (commit e66ed85). This grants the exemption to
blocks/modules only after they are marked for blacklist. This change
makes sure that the id exemption is used only to prevent the
blacklisting bug. The blocks/modules will still go through other forms
of id validation such as the naming convention check.

Signed-off-by: Solomon Tan <solomonbstoner@yahoo.com.au>
willcode pushed a commit to willcode/gnuradio that referenced this pull request May 15, 2021
I refer to the mailing list thread on May 8 2021 regarding "Embedded
Python Block Tutorial Param ID Error".

The bug is caused by gnuradio#4522 as names of embedded python blocks
such as `epy_block_0` are blacklisted because of "blocks" in the
argument `black_listed_ids`. It affects both embedded python blocks and
modules. This commit fixes the bug by granting exemption for these two
types of blocks from id validation.

This commit makes the regex more strictly follow the naming convention
of epy blocks and modules so that only these will be fast-tracked
through the id validation. The naming convention appears to be a "epy"
prefix, followed by "block" or "module", and a number suffix.

This regex also takes into account the user copying-and-pasting the epy
blocks resulting in id names like `epy_block_0_0` and `epy_block_0_0_0`.

Instead of checking for epy blocks or modules using the naming
convention in the id like commit ac383d1 does, this commit checks by
looking at the block's key. All epy blocks and modules are identified
with the key `epy_block` and `epy_module` respectively.

This commit fixes the bug by changing the import name such that it does
not coincide with the generated id for epy blocks and modules. This
means that there no longer needs to be an exception case in the
validation id function for epy blocks and modules.

This commit fixes the bug by giving epy blocks and modules a new
attribute that grants them exemption from blacklist id validation.
Unlike v1, v2 and v3, this does not depend on the naming convention of
the block ids or block keys, which could change in the future, making
the code less of a hassle to maintain.

Improved on v5 (commit b85da80) with the following changes.
1. Changed `exempt_from_id_validation` from an instance variable to a
class variable since all instances of epy blocks and modules are
affected by the bug mentioned in v5.
2. Changed `hasattr` to `getattr` to prevent the bug where a block is
exempted from id validation when `exempt_from_id_validation=False`.

Improved from v6 (commit e66ed85). This grants the exemption to
blocks/modules only after they are marked for blacklist. This change
makes sure that the id exemption is used only to prevent the
blacklisting bug. The blocks/modules will still go through other forms
of id validation such as the naming convention check.

Signed-off-by: Solomon Tan <solomonbstoner@yahoo.com.au>
(cherry picked from commit 4870dcf)
Signed-off-by: Jeff Long <willcode4@gmail.com>
willcode pushed a commit to willcode/gnuradio that referenced this pull request May 15, 2021
I refer to the mailing list thread on May 8 2021 regarding "Embedded
Python Block Tutorial Param ID Error".

The bug is caused by gnuradio#4522 as names of embedded python blocks
such as `epy_block_0` are blacklisted because of "blocks" in the
argument `black_listed_ids`. It affects both embedded python blocks and
modules. This commit fixes the bug by granting exemption for these two
types of blocks from id validation.

This commit makes the regex more strictly follow the naming convention
of epy blocks and modules so that only these will be fast-tracked
through the id validation. The naming convention appears to be a "epy"
prefix, followed by "block" or "module", and a number suffix.

This regex also takes into account the user copying-and-pasting the epy
blocks resulting in id names like `epy_block_0_0` and `epy_block_0_0_0`.

Instead of checking for epy blocks or modules using the naming
convention in the id like commit ac383d1 does, this commit checks by
looking at the block's key. All epy blocks and modules are identified
with the key `epy_block` and `epy_module` respectively.

This commit fixes the bug by changing the import name such that it does
not coincide with the generated id for epy blocks and modules. This
means that there no longer needs to be an exception case in the
validation id function for epy blocks and modules.

This commit fixes the bug by giving epy blocks and modules a new
attribute that grants them exemption from blacklist id validation.
Unlike v1, v2 and v3, this does not depend on the naming convention of
the block ids or block keys, which could change in the future, making
the code less of a hassle to maintain.

Improved on v5 (commit b85da80) with the following changes.
1. Changed `exempt_from_id_validation` from an instance variable to a
class variable since all instances of epy blocks and modules are
affected by the bug mentioned in v5.
2. Changed `hasattr` to `getattr` to prevent the bug where a block is
exempted from id validation when `exempt_from_id_validation=False`.

Improved from v6 (commit e66ed85). This grants the exemption to
blocks/modules only after they are marked for blacklist. This change
makes sure that the id exemption is used only to prevent the
blacklisting bug. The blocks/modules will still go through other forms
of id validation such as the naming convention check.

Signed-off-by: Solomon Tan <solomonbstoner@yahoo.com.au>
(cherry picked from commit 4870dcf)
Signed-off-by: Jeff Long <willcode4@gmail.com>
willcode pushed a commit that referenced this pull request May 15, 2021
I refer to the mailing list thread on May 8 2021 regarding "Embedded
Python Block Tutorial Param ID Error".

The bug is caused by #4522 as names of embedded python blocks
such as `epy_block_0` are blacklisted because of "blocks" in the
argument `black_listed_ids`. It affects both embedded python blocks and
modules. This commit fixes the bug by granting exemption for these two
types of blocks from id validation.

This commit makes the regex more strictly follow the naming convention
of epy blocks and modules so that only these will be fast-tracked
through the id validation. The naming convention appears to be a "epy"
prefix, followed by "block" or "module", and a number suffix.

This regex also takes into account the user copying-and-pasting the epy
blocks resulting in id names like `epy_block_0_0` and `epy_block_0_0_0`.

Instead of checking for epy blocks or modules using the naming
convention in the id like commit ac383d1 does, this commit checks by
looking at the block's key. All epy blocks and modules are identified
with the key `epy_block` and `epy_module` respectively.

This commit fixes the bug by changing the import name such that it does
not coincide with the generated id for epy blocks and modules. This
means that there no longer needs to be an exception case in the
validation id function for epy blocks and modules.

This commit fixes the bug by giving epy blocks and modules a new
attribute that grants them exemption from blacklist id validation.
Unlike v1, v2 and v3, this does not depend on the naming convention of
the block ids or block keys, which could change in the future, making
the code less of a hassle to maintain.

Improved on v5 (commit b85da80) with the following changes.
1. Changed `exempt_from_id_validation` from an instance variable to a
class variable since all instances of epy blocks and modules are
affected by the bug mentioned in v5.
2. Changed `hasattr` to `getattr` to prevent the bug where a block is
exempted from id validation when `exempt_from_id_validation=False`.

Improved from v6 (commit e66ed85). This grants the exemption to
blocks/modules only after they are marked for blacklist. This change
makes sure that the id exemption is used only to prevent the
blacklisting bug. The blocks/modules will still go through other forms
of id validation such as the naming convention check.

Signed-off-by: Solomon Tan <solomonbstoner@yahoo.com.au>
(cherry picked from commit 4870dcf)
Signed-off-by: Jeff Long <willcode4@gmail.com>
willcode pushed a commit that referenced this pull request May 15, 2021
I refer to the mailing list thread on May 8 2021 regarding "Embedded
Python Block Tutorial Param ID Error".

The bug is caused by #4522 as names of embedded python blocks
such as `epy_block_0` are blacklisted because of "blocks" in the
argument `black_listed_ids`. It affects both embedded python blocks and
modules. This commit fixes the bug by granting exemption for these two
types of blocks from id validation.

This commit makes the regex more strictly follow the naming convention
of epy blocks and modules so that only these will be fast-tracked
through the id validation. The naming convention appears to be a "epy"
prefix, followed by "block" or "module", and a number suffix.

This regex also takes into account the user copying-and-pasting the epy
blocks resulting in id names like `epy_block_0_0` and `epy_block_0_0_0`.

Instead of checking for epy blocks or modules using the naming
convention in the id like commit ac383d1 does, this commit checks by
looking at the block's key. All epy blocks and modules are identified
with the key `epy_block` and `epy_module` respectively.

This commit fixes the bug by changing the import name such that it does
not coincide with the generated id for epy blocks and modules. This
means that there no longer needs to be an exception case in the
validation id function for epy blocks and modules.

This commit fixes the bug by giving epy blocks and modules a new
attribute that grants them exemption from blacklist id validation.
Unlike v1, v2 and v3, this does not depend on the naming convention of
the block ids or block keys, which could change in the future, making
the code less of a hassle to maintain.

Improved on v5 (commit b85da80) with the following changes.
1. Changed `exempt_from_id_validation` from an instance variable to a
class variable since all instances of epy blocks and modules are
affected by the bug mentioned in v5.
2. Changed `hasattr` to `getattr` to prevent the bug where a block is
exempted from id validation when `exempt_from_id_validation=False`.

Improved from v6 (commit e66ed85). This grants the exemption to
blocks/modules only after they are marked for blacklist. This change
makes sure that the id exemption is used only to prevent the
blacklisting bug. The blocks/modules will still go through other forms
of id validation such as the naming convention check.

Signed-off-by: Solomon Tan <solomonbstoner@yahoo.com.au>
(cherry picked from commit 4870dcf)
Signed-off-by: Jeff Long <willcode4@gmail.com>
mormj pushed a commit to mormj/gnuradio that referenced this pull request Nov 22, 2021
I refer to the mailing list thread on May 8 2021 regarding "Embedded
Python Block Tutorial Param ID Error".

The bug is caused by gnuradio#4522 as names of embedded python blocks
such as `epy_block_0` are blacklisted because of "blocks" in the
argument `black_listed_ids`. It affects both embedded python blocks and
modules. This commit fixes the bug by granting exemption for these two
types of blocks from id validation.

This commit makes the regex more strictly follow the naming convention
of epy blocks and modules so that only these will be fast-tracked
through the id validation. The naming convention appears to be a "epy"
prefix, followed by "block" or "module", and a number suffix.

This regex also takes into account the user copying-and-pasting the epy
blocks resulting in id names like `epy_block_0_0` and `epy_block_0_0_0`.

Instead of checking for epy blocks or modules using the naming
convention in the id like commit ac383d1 does, this commit checks by
looking at the block's key. All epy blocks and modules are identified
with the key `epy_block` and `epy_module` respectively.

This commit fixes the bug by changing the import name such that it does
not coincide with the generated id for epy blocks and modules. This
means that there no longer needs to be an exception case in the
validation id function for epy blocks and modules.

This commit fixes the bug by giving epy blocks and modules a new
attribute that grants them exemption from blacklist id validation.
Unlike v1, v2 and v3, this does not depend on the naming convention of
the block ids or block keys, which could change in the future, making
the code less of a hassle to maintain.

Improved on v5 (commit b85da80) with the following changes.
1. Changed `exempt_from_id_validation` from an instance variable to a
class variable since all instances of epy blocks and modules are
affected by the bug mentioned in v5.
2. Changed `hasattr` to `getattr` to prevent the bug where a block is
exempted from id validation when `exempt_from_id_validation=False`.

Improved from v6 (commit e66ed85). This grants the exemption to
blocks/modules only after they are marked for blacklist. This change
makes sure that the id exemption is used only to prevent the
blacklisting bug. The blocks/modules will still go through other forms
of id validation such as the naming convention check.

Signed-off-by: Solomon Tan <solomonbstoner@yahoo.com.au>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ported-to-3.8 Has been ported to 3.8 ported-to-3.9 Has been ported to 3.9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants