-
Notifications
You must be signed in to change notification settings - Fork 8
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
Feet And Inches 1.0 #37
Open
Randall-Scharpf
wants to merge
53
commits into
Wendelstein7:master
Choose a base branch
from
Randall-Scharpf:feetandinches_1.0
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Feet And Inches 1.0 #37
Randall-Scharpf
wants to merge
53
commits into
Wendelstein7:master
from
Randall-Scharpf:feetandinches_1.0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
create numSigFigs() use function to set flag for number of significant figures
create numSigFigs() use function to set flag for number of significant figures rebase to beta
make the old test cases actually follow sig figs
…harpf/DiscordUnitCorrector into significantdigits_1.0
this behavior is used with temperature scales allows better expandability fixes bugs related to NaN and zero
stophittingyourselfstophittingyourselfstophittingyourself
stophittingyourselfstophittingyourselfstophittingyourself
…harpf/DiscordUnitCorrector into significantdigits_1.0
note to self dont rebase in the wrong direction, its painful
…f/DiscordUnitCorrector into feetandinches_1.0
fixed a bug related to that test case added a second test case that also catches that bug
this is more colloquial
should rewrite test suite to test all locales
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #22
The head of this PR is a direct future from the head of #27, so that issue should probably be merged first. Similarly, if there are issues with either PR, they can both be corrected on this request.
A considerable number of test cases has been given, considering the complexity and the density of special cases in this pull request.
It is also worth noting that because of the degree of complexity of this feature, the number of data structures in the code has multiplied, the number of functions has ballooned, and the simplicity of many functions has been lost. The test cases still run in only a few milliseconds, so the observed efficiency hit is small, but it is not negligible and this implementation should not be claimed to be optimal. However, this code is fully functional, so readability-enhancing and maintainability-enhancing refactors would, at this point, be simply extra. The same is true of any efficiency-improving refactors.
Currently, all the unit tests of this portion use input strings and output strings from the main method in
unitconversions.py
. It is worth considering to add test cases for the submethods, or to add test cases that only inspect certain aspects of the input and output strings, but again, at this point I believe, due to the full functionality of this feature, that such tests would be simply extra.In any case, here's Feet And Inches 1,0!