Skip to content
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

Add Convert Number to base-10 node #1621

Merged
merged 2 commits into from
Mar 6, 2023

Conversation

JustNoon
Copy link
Contributor

@JustNoon JustNoon commented Mar 3, 2023

Small node to go from any text to a base-10 integer.

Criticism and/ or improvements welcome.

backend/src/nodes/nodes/utility/convert_number.py Outdated Show resolved Hide resolved
backend/src/nodes/nodes/utility/convert_number.py Outdated Show resolved Hide resolved
backend/src/nodes/nodes/utility/convert_number.py Outdated Show resolved Hide resolved
@JustNoon
Copy link
Contributor Author

JustNoon commented Mar 4, 2023

Thank you for the review. I appreciate the explanations too. It's been a while for python for me

@RunDevelopment
Copy link
Member

I wonder whether we should call the node "Parse Number" instead. "Convert" is quite generic. Thoughts?

@JustNoon
Copy link
Contributor Author

JustNoon commented Mar 5, 2023

I also prefer a more technical name like parse but when i was workshopping it in discord there was a general reticence towards programmer jargon even if they're more correct.

I like Parse Number a lot more than convert. Makes it easier to quick search for too.

@joeyballentine
Copy link
Member

Maybe "extract number"?

@JustNoon
Copy link
Contributor Author

JustNoon commented Mar 5, 2023

"Extract" feels more like a substring manipulation. It'd feel even weirder imo if/ when there's actual string extraction.

Parse is still my vote on verb. It's unambiguous and doesn't overlap with other nodes.

Reviewed by: Michael Schmidt <msrd0000@gmail.com>
Co-authored-by: Michael Schmidt <msrd0000@gmail.com>
@joeyballentine joeyballentine merged commit 33e81fe into chaiNNer-org:main Mar 6, 2023
@JustNoon JustNoon deleted the base-10 branch March 6, 2023 19:03
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