Skip to content

Conversation

aplavin
Copy link
Contributor

@aplavin aplavin commented Feb 27, 2025

Following up on #162. I already found myself copy-pasting this function several times, would be nice to have it here in MCM.

Better naming suggestions welcome :)

Copy link

codecov bot commented Feb 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.80%. Comparing base (0455a9c) to head (040eed0).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #173      +/-   ##
==========================================
+ Coverage   89.46%   89.80%   +0.33%     
==========================================
  Files          20       21       +1     
  Lines        1168     1187      +19     
==========================================
+ Hits         1045     1066      +21     
+ Misses        123      121       -2     
Flag Coverage Δ
unittests 89.80% <100.00%> (+0.33%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@baggepinnen
Copy link
Owner

Thanks for the PR :)
Any name without the awkward character would be good, bymap2, bymap_a etc.

@aplavin
Copy link
Contributor Author

aplavin commented Feb 27, 2025

renamed!

@aplavin
Copy link
Contributor Author

aplavin commented Jul 17, 2025

any more thoughts on how best to proceed here?
I'd be quite happy to stop copy-pasting this every once in a while :)

@baggepinnen
Copy link
Owner

This functionality is sufficiently complex that I cannot look at the implementation and determine when it works and when it doesn't. It would be nice to have a few more test cases to make sure it doesn't produce the wrong result for complex data structures. There are a few test cases here

it's okay if it doesn't handle all cases, but we should at least ensure that it errors rather than returning the wrong result

@aplavin
Copy link
Contributor Author

aplavin commented Jul 28, 2025

Makes sense! Added more tests.

@baggepinnen baggepinnen merged commit fe8a690 into baggepinnen:master Jul 28, 2025
4 checks passed
@aplavin aplavin deleted the patch-1 branch July 29, 2025 14:06
@aplavin aplavin mentioned this pull request Jul 29, 2025
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