Sort conversions by result magnitude

[aaronkollasch]

This PR sorts conversions to prioritize the results that are closest to 0 in log-space.

While alfred-converter reduces unhelpful conversions with a limit on the maximum magnitude difference between the from_ and to units, I think it's actually better to sort based on the magnitude of the resulting conversion.

Screenshots:
Before, the conversion of 100 s did not show the conversion to minutes in the top 9.

After, 100 second = 1.66666667 minutes is the top result.

(note that I've removed some obscure time units that popped up in the second screenshot: the "hundred second" and the "ten milli second")

[gemini-code-assist]

Hello @aaronkollasch, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello team, gemini-code-assist here to provide a summary of this pull request. This PR aims to improve the user experience of the unit converter by changing how conversion results are sorted. Previously, results might have been sorted based on the magnitude difference between the input and output units. This change introduces a new sorting logic that prioritizes conversion results whose absolute magnitude is closest to 1 (which corresponds to a log-space magnitude closest to 0). The author provides screenshots demonstrating how this change makes more intuitive conversions (like seconds to minutes) appear higher in the list.

Highlights

• Improved Sorting Logic: The core change is the introduction of a new sorting mechanism for conversion results. Instead of relying on the magnitude difference between the input and output units, the results are now sorted based on the absolute magnitude of the converted quantity itself, prioritizing values closest to 1.
• New Sorting Function: A new helper function, sort_abs_magnitude, is introduced to calculate the sorting key for each conversion result. This function determines the absolute magnitude (in log10 space) of the converted value.
• Sorting Implementation: The list of potential conversion results is now explicitly collected and sorted using the new sort_abs_magnitude function before being processed and displayed.

Changelog

• converter/convert.py
  • Added sort_abs_magnitude function (lines 463-476) to calculate the absolute log-magnitude of a conversion result for sorting.
  • Modified the main function (lines 485-486) to collect conversion results into a list and sort it using the new sort_abs_magnitude function before iterating.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

Units convert with grace,
Magnitude finds its place,
Closest to one is best,
Putting results to test.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a new sorting mechanism for conversion results, aiming to prioritize results with magnitudes closest to zero in log-space. This is a good user experience improvement.

The implementation involves a new sort_abs_magnitude function. I've identified two critical issues within this function related to handling zero values during logarithm calculation and processing non-conversion inputs (e.g., plain numbers) that could lead to runtime errors. Addressing these will make the new sorting robust.

No explicit style guide was provided, so feedback adheres to general Python best practices and PEP 8, which the code largely follows.

Summary of Findings

• Runtime Error: Logarithm of Zero: The sort_abs_magnitude function does not safely handle new_quantity being zero before calling .log10(). This can lead to a decimal.DivisionByZero exception if the default decimal context traps this operation, which would break the sorting process. This was commented on.
• Runtime Error: Handling Non-Conversion Inputs: The sort_abs_magnitude function does not account for (None, quantity, None) tuples that can be produced by units.convert(). When from_ is None, accessing from_.to_base() will cause an AttributeError, breaking the sort. This was commented on.
• Type Hint Precision: In sort_abs_magnitude, the type hint for new_quantity (line 468) is _FractionDecimalStr. Given that to.from_base is annotated to return _FractionDecimal, the hint for new_quantity could be more precise (_FractionDecimal). This is a minor point and was not commented on due to review settings.
• Missing Type Hint for Parameter: The result parameter of sort_abs_magnitude (line 463) lacks a type hint. Adding one, such as result: tuple[Optional[Unit], _FractionDecimalStr, Optional[Unit]] (after importing Optional from typing and using forward reference 'Unit'), would improve clarity. This is a minor point and was not commented on due to review settings.

Merge Readiness

The pull request introduces a valuable feature for sorting conversion results. However, there are critical issues in the sort_abs_magnitude function related to error handling that must be addressed before merging to ensure stability. Once these issues are resolved, the PR should be in good shape. As an AI, I am not authorized to approve pull requests; please ensure further review and approval by authorized maintainers after the changes are made.

[wolph]

There's just one caveat to this change.
Alfred has 2 options for sorting:

1. sort by priority based on what you use most often. This is especially useful for unit conversions where you oftentimes do the same conversions.
2. manual sort as suggested by the workflow (what we're doing here). Much more useful for maths, much less useful for unit conversions.

I'm certainly not against this change since it seems really useful, but it would be nice to still have a smart sorting for the units. I'll have to check if there's an option to have both somehow.

[aaronkollasch]

Yeah true! I actually ran into 1. when testing the change. After pressing enter to copy a conversion to minutes, I was confused as to why it kept getting put on the top of the list, even when it should've been much further down. It only reset when I went into the Alfred advanced preferences and hit "Clear Knowledge".

So currently sorting by priority overrides the manual sorting, which probably makes sense if there's no way to do both.

Even if we keep Alfred's priority sort, in most cases this change will still help as it will surface relevant conversions that might otherwise be buried outside of the top 9.