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

Refs #2615 - Convert create_py_tags.py to Python 3 #2630

Closed
wants to merge 1 commit into from

Conversation

claudep
Copy link
Contributor

@claudep claudep commented Oct 20, 2020

No description provided.

@claudep
Copy link
Contributor Author

claudep commented Oct 20, 2020

Initial attempt. The resulting .tags file should be carefully inspected and tested, of course.

Copy link
Member

@eht16 eht16 left a comment

Choose a reason for hiding this comment

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

Thanks!
A few remarks:

  • scripts/create_py_tags.py:26: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative usesscripts/create_py_tags.py:26: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative useswe should useimportlib` as the message tells
  • the #--------------... seperators should be either # --------------... or removed completely (I'm the one who introduced them initially but I'm fine with removing them)
  • ideally the script is compatible with PEP8

As you said, the resulting .tags file should be revised. After a quick look, we actually loose some information because the custom _formatargspec() implementation doesn't work anymore. It errors out with a TypeError which we silently ignore (I manually checked with pprint.pformat() as an example).
I think we should refactor this code to use inspect.Signature which was probably not available when the current implementation was made (which was more than ten years ago). But this is going to be out of scope of this PR, so it's fine to fix this later on.

# some modules execute funky code when they are imported which we really don't want here
# (though if you feel funny, try: 'import antigravity')
PYTHON_LIB_IGNORE_MODULES = (u'antigravity.py', u'idlelib/idle.py', u'multiprocessing/util.py')
PYTHON_LIB_IGNORE_MODULES = ('antigravity.py', 'idlelib/idle.py', 'multiprocessing/util.py')
Copy link
Member

Choose a reason for hiding this comment

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

Additionally, we should exclude idlelib/__main__.py as otherwise the Idle IDE is started.
And we could even exclude this.py as it contains only the Zen of Python but no tags. So we save us printing the Zen of Python.


PYTHON_LIB_DIRECTORY = os.path.dirname(os.__file__)
PYTHON_LIB_IGNORE_PACKAGES = (u'test', u'dist-packages', u'site-packages', 'Tools')
PYTHON_LIB_IGNORE_PACKAGES = ('test', 'dist-packages', 'site-packages', 'Tools')
Copy link
Member

Choose a reason for hiding this comment

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

If we also exclude the config-3.9-x86_64-linux-gnu package (which might be named differently per system), we avoid an irritating usage message:
Usage: scripts/create_py_tags.py [--prefix|--exec-prefix|--includes|--libs|--cflags|--ldflags|--extension-suffix|--help|--abiflags|--configdir|--embed]

We can read the config module name and strip the prefix part of the part to get the module name, maybe like so:

# read the python-config path from LIBPL and strip the prefix part
# (e.g. /usr/lib/python3.8/config-3.8-x86_64-linux-gnu gets config-3.8-x86_64-linux-gnu)
PYTHON_CONFIG_DIR = sysconfig.get_config_var('LIBPL')
PYTHON_CONFIG_PACKAGE = PYTHON_CONFIG_DIR[len(PYTHON_LIB_DIRECTORY)+1:] \
    if PYTHON_CONFIG_DIR.startswith(PYTHON_LIB_DIRECTORY) else PYTHON_CONFIG_DIR
PYTHON_LIB_IGNORE_PACKAGES = ('test', 'dist-packages', 'site-packages', 'Tools', 
                              PYTHON_CONFIG_PACKAGE)

parser.process_file(filename)
try:
parser.process_file(filename)
except (SystemExit, ImportError, TypeError):
Copy link
Member

Choose a reason for hiding this comment

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

Why catching SystemExit and ImportError? They do not occur, at least not for me.
The TypeError exceptions seem rather like an error we should handle instead of silently ignoring.

In either case, I think we should print the error instead of silencing it:

        except TypeError as exc:
            print(f'{exc.__class__.__name__} in {filename}: {exc}')

this gives: TypeError in /home/enrico/.pyenv/versions/3.9.0/lib/python3.9/http/cookies.py: can only concatenate list (not "tuple") to list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably I got those errors at some point when working on this. You can always try to remove them and you will see if anyone complain in the future.

@eht16
Copy link
Member

eht16 commented Sep 16, 2021

@claudep are you still interested in working on this?

@claudep
Copy link
Contributor Author

claudep commented Sep 17, 2021

I'm not sure I'll be able to find time for that, very sorry, Feel free to continue and take over the patch.

@eht16
Copy link
Member

eht16 commented Sep 21, 2021

Alright, thanks for your feedback and initial work! I'm going to complete this.

@eht16
Copy link
Member

eht16 commented May 21, 2023

Done in #3039

@eht16 eht16 closed this May 21, 2023
@eht16 eht16 moved this from In Progress to Done in Remove Python2 use May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants