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

NEP deprecate 3.8; remove parallel arguments #24

Merged
merged 4 commits into from
Jan 29, 2024

Conversation

CodyCBakerPhD
Copy link
Member

Something's up with coverage report: https://app.codecov.io/github/catalystneuro/tqdm_publisher

Only calculating coverage of the 'tests' folder...

Also apparently company will be following NEP not python deprecation cycles so dropping 3.8 here; also removing some unneeded arguments to pytest in an effort to debug this

@CodyCBakerPhD CodyCBakerPhD self-assigned this Jan 29, 2024
@CodyCBakerPhD
Copy link
Member Author

@garrettmflynn Could you do me a favor and run the pytest with coverage flag locally and see what the resulting XML file looks like? (it should have hit counters for various lines in source)

@garrettmflynn
Copy link
Collaborator

<?xml version="1.0" ?>
<coverage version="7.4.0" timestamp="1706557588055" lines-valid="69" lines-covered="67" line-rate="0.971" branches-covered="0" branches-valid="0" branch-rate="0" complexity="0">
	<!-- Generated by coverage.py: https://coverage.readthedocs.io/en/7.4.0 -->
	<!-- Based on https://raw.githubusercontent.com/cobertura/web/master/htdocs/xml/coverage-04.dtd -->
	<sources>
		<source>/Users/garrettflynn/Documents/GitHub/progress-bars</source>
	</sources>
	<packages>
		<package name="src.tqdm_publisher" line-rate="0.9706" branch-rate="0" complexity="0">
			<classes>
				<class name="__init__.py" filename="src/tqdm_publisher/__init__.py" complexity="0" line-rate="1" branch-rate="0">
					<methods/>
					<lines>
						<line number="1" hits="1"/>
					</lines>
				</class>
				<class name="publisher.py" filename="src/tqdm_publisher/publisher.py" complexity="0" line-rate="0.9524" branch-rate="0">
					<methods/>
					<lines>
						<line number="1" hits="1"/>
						<line number="2" hits="1"/>
						<line number="4" hits="1"/>
						<line number="7" hits="1"/>
						<line number="8" hits="1"/>
						<line number="9" hits="1"/>
						<line number="10" hits="1"/>
						<line number="13" hits="1"/>
						<line number="14" hits="1"/>
						<line number="16" hits="1"/>
						<line number="17" hits="1"/>
						<line number="19" hits="1"/>
						<line number="21" hits="1"/>
						<line number="53" hits="1"/>
						<line number="54" hits="1"/>
						<line number="55" hits="1"/>
						<line number="57" hits="1"/>
						<line number="92" hits="1"/>
						<line number="93" hits="0"/>
						<line number="95" hits="1"/>
						<line number="96" hits="1"/>
					</lines>
				</class>
				<class name="testing.py" filename="src/tqdm_publisher/testing.py" complexity="0" line-rate="1" branch-rate="0">
					<methods/>
					<lines>
						<line number="1" hits="1"/>
						<line number="2" hits="1"/>
						<line number="5" hits="1"/>
						<line number="6" hits="1"/>
						<line number="9" hits="1"/>
						<line number="10" hits="1"/>
						<line number="11" hits="1"/>
						<line number="12" hits="1"/>
						<line number="14" hits="1"/>
						<line number="15" hits="1"/>
						<line number="16" hits="1"/>
						<line number="18" hits="1"/>
					</lines>
				</class>
			</classes>
		</package>
		<package name="tests" line-rate="0.9714" branch-rate="0" complexity="0">
			<classes>
				<class name="test_basic.py" filename="tests/test_basic.py" complexity="0" line-rate="0.9714" branch-rate="0">
					<methods/>
					<lines>
						<line number="1" hits="1"/>
						<line number="3" hits="1"/>
						<line number="5" hits="1"/>
						<line number="6" hits="1"/>
						<line number="9" hits="1"/>
						<line number="10" hits="1"/>
						<line number="11" hits="1"/>
						<line number="15" hits="1"/>
						<line number="16" hits="1"/>
						<line number="17" hits="1"/>
						<line number="19" hits="1"/>
						<line number="22" hits="1"/>
						<line number="23" hits="1"/>
						<line number="25" hits="1"/>
						<line number="27" hits="1"/>
						<line number="29" hits="1"/>
						<line number="30" hits="1"/>
						<line number="32" hits="1"/>
						<line number="33" hits="1"/>
						<line number="34" hits="1"/>
						<line number="37" hits="1"/>
						<line number="40" hits="1"/>
						<line number="41" hits="1"/>
						<line number="43" hits="1"/>
						<line number="45" hits="1"/>
						<line number="46" hits="1"/>
						<line number="49" hits="1"/>
						<line number="50" hits="1"/>
						<line number="51" hits="0"/>
						<line number="53" hits="1"/>
						<line number="54" hits="1"/>
						<line number="55" hits="1"/>
						<line number="56" hits="1"/>
						<line number="57" hits="1"/>
						<line number="58" hits="1"/>
					</lines>
				</class>
			</classes>
		</package>
	</packages>
</coverage>

@CodyCBakerPhD
Copy link
Member Author

Weird it's not ignoring tests/test_basic.py, can you mess with the codecov.yml or other method until it's properly ignoring that?

@garrettmflynn
Copy link
Collaborator

garrettmflynn commented Jan 29, 2024

Easiest way is to scope the coverage report tosrc: pytest -rsx --cov=./src --cov-report xml:./codecov.xml.

Running pytest-cov doesn't factor in the codecov.yml file.

@garrettmflynn
Copy link
Collaborator

That Codecov wouldn't even consider src, though, is very weird

@garrettmflynn
Copy link
Collaborator

How should I trigger an upload to Codecov to test these changes?

@CodyCBakerPhD
Copy link
Member Author

Ohhh I see

We usually do --cov=/. and then rely on the yaml to sort out what to ignore and such https://github.com/catalystneuro/neuroconv/blob/main/.github/workflows/testing.yml#L132

@CodyCBakerPhD
Copy link
Member Author

How should I trigger an upload to Codecov to test these changes?

I've added it to this PR - once this is merged I can trigger it on main through the actions

Copy link
Collaborator

@garrettmflynn garrettmflynn left a comment

Choose a reason for hiding this comment

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

Got it. Looks good to me!

@CodyCBakerPhD
Copy link
Member Author

CodyCBakerPhD commented Jan 29, 2024

Got it. Looks good to me!

Hah, your default GitHub PR review interface was set to 'changes requested' from the other GUIDE PR 😆 (or accidentally selected the wrong one)

@CodyCBakerPhD CodyCBakerPhD enabled auto-merge (squash) January 29, 2024 20:58
Copy link
Collaborator

@garrettmflynn garrettmflynn left a comment

Choose a reason for hiding this comment

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

Whoops! Alright, let's do it for real this time.

@CodyCBakerPhD CodyCBakerPhD merged commit e161cb9 into main Jan 29, 2024
17 of 18 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the clean_testing_workflow branch January 29, 2024 21:52
@CodyCBakerPhD
Copy link
Member Author

OK so long story short; the CI was 'passing' since it was first added, but was actually failing the entire time. There was a specific flag I had to set to make it fail in the CI if the upload failed

Otherwise, it does not play well with the global upload token; it rather seems to be keyed specifically to an individual repo

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