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 Test Coverage for Transformers import #348

Conversation

abarbosa94
Copy link
Collaborator

Description

  • This PR adds a simple set of unit tests to properly cover transformers' import use cases

@aaarrti
Copy link
Collaborator

aaarrti commented Mar 31, 2024

Do I understand correctly, that you're using pytest-mock to simulate the case when transformers package is installed / is not installed? I'd recommend to create different tox environments for this, instead of mocking parts of pythons import logic.


reload(pytorch_model)
# Mock the model's behavior
model_instance = PyTorchModel(model=mocker.MagicMock(spec=base_class))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @abarbosa94,

could you please make sure the original state is restored after this function exits? This can be done using context-manager fixture, e.g., https://github.com/understandable-machine-intelligence-lab/Quantus/blob/main/tests/metrics/conftest.py

@aaarrti
Copy link
Collaborator

aaarrti commented Apr 11, 2024

@abarbosa94, I believe that the test failures are caused by conflicting mocks. In case not, please don't spend too much time debugging those, i.e. this is a green light from me to just delete the failing tests (and unnecessary overcomplicated mocks)

@annahedstroem
Copy link
Member

annahedstroem commented Apr 15, 2024

@abarbosa94, I believe that the test failures are caused by conflicting mocks. In case not, please don't spend too much time debugging those, i.e. this is a green light from me to just delete the failing tests (and unnecessary overcomplicated mocks)

please see my answer on Slack! perhaps easiest and safest if you @aaarrti delete those mocks and dependencies? perhaps just push to this PR

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.

3 participants