Skip to content

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Aug 7, 2023

This caused pymc-experimental to fail as the test is used there but there is no access to the fixture.
The fixture was added after the changes in #6799
I don't think we need it?

CC @aerubanov


📚 Documentation preview 📚: https://pymc--6848.org.readthedocs.build/en/6848/

@ricardoV94 ricardoV94 added release maintenance no releasenotes Skipped in automatic release notes generation labels Aug 7, 2023
@ricardoV94 ricardoV94 changed the title Don't include seeded_test fixture in exported BaseTestDistributionRandom Do not use seeded_test fixture in exported BaseTestDistributionRandom Aug 7, 2023
@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Merging #6848 (0f713e5) into main (ccad4c8) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6848      +/-   ##
==========================================
- Coverage   92.05%   92.03%   -0.02%     
==========================================
  Files          96       96              
  Lines       16369    16369              
==========================================
- Hits        15068    15065       -3     
- Misses       1301     1304       +3     
Files Changed Coverage Δ
pymc/testing.py 91.46% <100.00%> (ø)

... and 1 file with indirect coverage changes

@ricardoV94 ricardoV94 force-pushed the fix_BaseTestDistributionRandom branch from 93aa347 to bc91b3c Compare August 7, 2023 10:15
@ricardoV94 ricardoV94 force-pushed the fix_BaseTestDistributionRandom branch from bc91b3c to 0f713e5 Compare August 7, 2023 10:58
@aerubanov
Copy link
Contributor

@ricardoV94 Yeah, looks like seeded_test fixture is actually is not needed here.

@ricardoV94 ricardoV94 requested a review from twiecki August 8, 2023 05:46
Copy link
Member

@jessegrabowski jessegrabowski left a comment

Choose a reason for hiding this comment

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

LGTM. The tests even seem like they were written to be like this in the first place (judging from the partial functions that pass in the random state)

@ricardoV94 ricardoV94 merged commit d59a960 into pymc-devs:main Aug 8, 2023
@ricardoV94 ricardoV94 added bug and removed no releasenotes Skipped in automatic release notes generation maintenance labels Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants