Skip to content

Comments

added check to Reference.plate_map to raise exception if the plate name is not found in the reference#38

Merged
cgevans merged 6 commits intomainfrom
error-if-plate-name-not-found-in-reference
May 15, 2025
Merged

added check to Reference.plate_map to raise exception if the plate name is not found in the reference#38
cgevans merged 6 commits intomainfrom
error-if-plate-name-not-found-in-reference

Conversation

@dave-doty
Copy link
Collaborator

@dave-doty dave-doty commented May 14, 2025

I'm hoping this can also be merged into other "active" branches such as echo. We got bit by this recently, where the prior behavior was that it simply output an empty plate, and it took some tracking to figure out why; this error would have helped find that problem sooner.

@dave-doty dave-doty marked this pull request as draft May 14, 2025 22:26
@dave-doty
Copy link
Collaborator Author

I converted to draft so I can write a unit test to test this; will do that later.

@codecov-commenter
Copy link

codecov-commenter commented May 14, 2025

Codecov Report

Attention: Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.

Project coverage is 79.41%. Comparing base (bdea4d1) to head (055324c).

Files with missing lines Patch % Lines
src/alhambra_mixes/references.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #38      +/-   ##
==========================================
+ Coverage   79.27%   79.41%   +0.13%     
==========================================
  Files          23       23              
  Lines        3132     3143      +11     
==========================================
+ Hits         2483     2496      +13     
+ Misses        649      647       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dave-doty dave-doty marked this pull request as ready for review May 14, 2025 22:40
Not really related to this PR, but it seemed like a good idea since Python 3.13 came out last October
@dave-doty
Copy link
Collaborator Author

dave-doty commented May 14, 2025

Hmm, I added a commit to bump the version of Python tested to include 3.13, but the tests only ran up to version 3.12, not sure why. I don't see any other configuration file that references Python versions. If there's a reason not to support 3.13 yet, obviously we can undo that before merging, but it seemed like a good idea to test Python 3.13 since it's been out since last October.

@dave-doty
Copy link
Collaborator Author

Note that I also changed docstrings in many files, but all the key logical changes in code are in references.py and test_references.py.

@cgevans
Copy link
Owner

cgevans commented May 15, 2025

Hmm, I added a commit to bump the version of Python tested to include 3.13, but the tests only ran up to version 3.12, not sure why. I don't see any other configuration file that references Python versions. If there's a reason not to support 3.13 yet, obviously we can undo that before merging, but it seemed like a good idea to test Python 3.13 since it's been out since last October.

I think that changing the github actions in a PR doesn't actually change the actions that are run; it needs to be on the main branch.

@cgevans cgevans merged commit 243f64d into main May 15, 2025
12 checks passed
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