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

Add missing import path for extractors #74

Merged

Conversation

edan-bainglass
Copy link
Member

Imported extractors module in main __init__.py to support from qe_tools.extractors import extract path needed by MaRDA extractors system.

@sphuber
Copy link
Contributor

sphuber commented Jan 15, 2024

Why is this necessary? Doing from qe_tools.extractors import extract without this works just fine. Not saying we shouldn't add this, don't think there is any harm, but just trying to understand what the problem was that they encountered?

@edan-bainglass
Copy link
Member Author

Why is this necessary? Doing from qe_tools.extractors import extract without this works just fine. Not saying we shouldn't add this, don't think there is any harm, but just trying to understand what the problem was that they encountered?

See https://github.com/marda-alliance/metadata_extractors_registry/pull/54/files#r1450396537

For some reason, they were unable to access the extractors module. My "fix" appears to have resolved the matter, but it is unclear what caused the issue in the first place. I was not planning do dig further unless an issue arises, as this is somewhat low priority. At least until the final round of testing, which requires some work on their end.

@edan-bainglass edan-bainglass merged commit ce7279c into aiidateam:main Feb 2, 2024
5 checks passed
@edan-bainglass
Copy link
Member Author

@sphuber I'm confused. The PR was all checkmarks. But once merged onto main, not a single test passed. What gives?

@sphuber
Copy link
Contributor

sphuber commented Feb 2, 2024

Well, the PR tests ran three weeks ago. But a lot can change in three weeks. Welcome to dependency management 😅

It looks like the latest builds used pytest==8.0.0 which was released last week. Since this is a major release, it breaks certain plugins. The solution is to pin the major version of pytest that we use in the dependencies. Something like pytest~=7.0.

@edan-bainglass
Copy link
Member Author

edan-bainglass commented Feb 2, 2024

I see. I'll pin pytest to ~7.0 as you suggest.

@edan-bainglass edan-bainglass deleted the marda-extractors-support branch February 2, 2024 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants