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

Cleanup regressions #132

Merged
merged 4 commits into from
Jun 5, 2023
Merged

Conversation

ntjohnson1
Copy link
Collaborator

Test if we can resolve this #131 (comment)

Revert some changes, enforce pylint, and fix a bug because windows path representation is silly.

@ntjohnson1
Copy link
Collaborator Author

Is there a reason we moved away from just calling coveralls directly? That coveralls github action has an issue open specifically related to it having issues with python. They point to this python specific package to support https://coveralls-python.readthedocs.io/en/latest/usage/configuration.html#github-actions-support

lemurheavy/coveralls-public#1610 (comment)

@ntjohnson1 ntjohnson1 marked this pull request as ready for review June 5, 2023 22:29
@dmdunla dmdunla merged commit 926b887 into sandialabs:main Jun 5, 2023
@ntjohnson1 ntjohnson1 deleted the cleanup_regressions branch June 5, 2023 22:30
@dmdunla
Copy link
Collaborator

dmdunla commented Jun 5, 2023

Here's an example of what's happening in coveralls when including the file docstring at the top; the class docstring appears as uncovered "code". If there is a problem with coveralls, we could consider moving to another code coverage tool; I am not too fluent in what's available.

@dmdunla
Copy link
Collaborator

dmdunla commented Jun 5, 2023

@ntjohnson1
Copy link
Collaborator Author

If I understand correctly it's our GitHub action integration with coveralls rather than coveralls itself. Feel free to assign an issue to me to investigate or I'll file one later this week.

@dmdunla
Copy link
Collaborator

dmdunla commented Jun 5, 2023

Here's the output of running coveralls directly in a clean conda env locally with these updates:

https://coveralls.io/jobs/122739175

This was run using the following (per coveralls docs):

pip install coveralls
coverage run --source pyttb -m pytest tests/ --packaging
COVERALLS_REPO_TOKEN=<removed> coveralls

@dmdunla
Copy link
Collaborator

dmdunla commented Jun 5, 2023

I could not get this to run with this level of coverage unless I created a new conda env. So, I think we may want to be more strict about our minimum versions of dependencies that we use for development.

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