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

fix: Add missing importlib.util import #278

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Add missing importlib.util import #278

wants to merge 1 commit into from

Conversation

matthewfeickert
Copy link
Contributor

@matthewfeickert matthewfeickert commented Dec 3, 2024 •
edited
Loading

Resolves #237

Context:

  • Without first importing importlib.util an AttributeError will be raised
Traceback (most recent call last):
  File "/madanalysis5/./bin/ma5", line 51, in <module>
    if not importlib.util.find_spec("six"):
           ^^^^^^^^^^^^^^
AttributeError: module 'importlib' has no attribute 'util'

Reproducer

# python --version
Python 3.11.10
# ./bin/ma5
Traceback (most recent call last):
  File "/madanalysis5/./bin/ma5", line 51, in <module>
    if not importlib.util.find_spec("six"):
           ^^^^^^^^^^^^^^
AttributeError: module 'importlib' has no attribute 'util'

Description of the Change:

Benefits:

Possible Drawbacks:

Related GitHub Issues:

@matthewfeickert matthewfeickert marked this pull request as ready for review December 3, 2024 18:09
@matthewfeickert
Copy link
Contributor Author

@jackaraz this is ready for review.

@BFuks
Copy link
Member

BFuks commented Dec 3, 2024

Hi @ matthewfeickert. This is actually already included in the upcoming release.

Cheers,

Benj

@matthewfeickert
Copy link
Contributor Author

matthewfeickert commented Dec 3, 2024 •
edited
Loading

This is actually already included in the upcoming release.

@BFuks How so? This isn't on any of the [non-stale] branches. Do you not do open development?

@BFuks
Copy link
Member

BFuks commented Dec 3, 2024 via email •
edited
Loading

@matthewfeickert
fix: Add missing importlib.util import
e42a7fd 
* Without first importing importlib.util an AttributeError will be raised

```
Traceback (most recent call last):
  File "/madanalysis5/./bin/ma5", line 51, in <module>
    if not importlib.util.find_spec("six"):
           ^^^^^^^^^^^^^^
AttributeError: module 'importlib' has no attribute 'util'
```
@matthewfeickert
Copy link
Contributor Author

matthewfeickert commented Dec 3, 2024 •
edited
Loading

Thanks @BFuks. Sorry I was only checking the branches that GitHub labels "active" (https://github.com/MadAnalysis/madanalysis5/branches/active) so I didn't see c9d1de8 as there is no associated PR for its branch.

Is there a projected timeline for a v1.10.17 release? At the moment this breaks all use.

I'm also not sure if you'd be open to getting a removal of six into v1.10.17 as well (c.f. Issue #279).

@BFuks
Copy link
Member

BFuks commented Dec 3, 2024

We are currently finalising it, with a paper planned within the next 2-3 months. I still need to implement a couple of functionalities (mostly automatic download from hepdata for some recast codes) and we are ready to go.

In fact you are correct, let's get rid of six entirely. I will add that in the next commit! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change in importlib
2 participants
@matthewfeickert @BFuks