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.

Sign up for GitHub

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

Jump to bottom

feat: add complex/parse-float64 #1362

Merged
merged 19 commits into from
Feb 28, 2024
Merged

feat: add complex/parse-float64 #1362

merged 19 commits into from
Feb 28, 2024

Conversation

Snehil-Shah
Copy link
Member

@Snehil-Shah Snehil-Shah commented Feb 24, 2024 •
edited
Loading

Resolves #1333

Description

What is the purpose of this pull request?

This pull request:

  • Adds the package @stdlib/complex/parse-float64.

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

Should it raise an error OR return null, when given a string that cannot be parsed into Complex128? Current implementation raises an error.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

Tests:

tests

Benchmarks

benchmarks

Examples

examples

Checklist

Please ensure the following tasks are completed before submitting this pull request.

@stdlib-js/reviewers

@Snehil-Shah
Add @stdlib/complex/parse-float64
8b8797c 
Signed-off-by: Snehil Shah <snehilshah.989@gmail.com>
stdlib-bot
stdlib-bot reviewed Feb 24, 2024
View reviewed changes
Copy link
Contributor

@stdlib-bot stdlib-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 Hi there! 👋

And thank you for opening your first pull request! We will review it shortly. 🏃 💨

@kgryte kgryte changed the title Add @stdlib/complex/parse-float64 feat: add complex/parse-float64 Feb 24, 2024
@kgryte kgryte added Feature Issue or pull request for adding a new feature. Utilities Issue or pull request concerning general utilities. labels Feb 24, 2024
Pranavchiku
Pranavchiku approved these changes Feb 24, 2024
View reviewed changes
Copy link
Member

@Pranavchiku Pranavchiku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Snehil-Shah! Looks good to me.

@Snehil-Shah
fix: failing tests
9d25909 
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah
fix: benchmarks
5ee83f7 
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah
fix: lint errors and suggested changes
fc7bd0a 
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah
fix: workflow lint errors
1c4e03d 
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah
test: add more varied test cases & examples
a4fe19f 
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@kgryte kgryte added the Needs Review A pull request which needs code review. label Feb 24, 2024
Pranavchiku
Pranavchiku requested changes Feb 24, 2024
View reviewed changes
Copy link
Member

@Pranavchiku Pranavchiku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested few changes.

lib/node_modules/@stdlib/complex/parse-float64/README.md Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/complex/parse-float64/docs/repl.txt Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/complex/parse-float64/examples/index.js Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/complex/parse-float64/lib/index.js Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/complex/parse-float64/test/test.js Outdated Show resolved Hide resolved
Snehil-Shah and others added 4 commits February 24, 2024 16:59
@Snehil-Shah
fix: suggested changes and add more test cases
72d223a 
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah @Pranavchiku
fix lint errors
1ee982f 
Co-authored-by: Pranav <85227306+Pranavchiku@users.noreply.github.com>
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah @Pranavchiku
Update lib/node_modules/@stdlib/complex/parse-float64/docs/repl.txt
a932f62 
Co-authored-by: Pranav <85227306+Pranavchiku@users.noreply.github.com>
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah
fix: lint suggestions
26956a4 
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@kgryte
Copy link
Member

kgryte commented Feb 25, 2024

Throwing an error is preferred for this API.

@kgryte kgryte added the Needs Changes Pull request which needs changes before being merged. label Feb 25, 2024
Snehil-Shah and others added 4 commits February 25, 2024 08:20
@Snehil-Shah @kgryte
Update lib/node_modules/@stdlib/complex/parse-float64/docs/types/test.ts
744d46b 
Co-authored-by: Athan <kgryte@gmail.com>
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah @kgryte
Update lib/node_modules/@stdlib/complex/parse-float64/docs/repl.txt
9a40758 
Co-authored-by: Athan <kgryte@gmail.com>
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah
fix: suggestions
c92f7d7 
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah
fix: ts tests
cf467ca
@Snehil-Shah
Copy link
Member Author

@kgryte did the required changes.

@Snehil-Shah
fix: conventional error messages
879f1a5 
Signed-off-by: Snehil Shah <130062020+Snehil-Shah@users.noreply.github.com>
@Snehil-Shah
Copy link
Member Author

@kgryte @Pranavchiku @Planeshifter any other suggestions on this PR?

@Planeshifter
refactor: use assertion package and format for error messages
37e3f55 
Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
Planeshifter
Planeshifter approved these changes Feb 28, 2024
View reviewed changes
Copy link
Member

@Planeshifter Planeshifter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get this over the finish line and landed. I made some small changes to the main.js file to follow how we format error messages by using the @stdlib/string/format package.

Will merge after CI has passed; thanks for your contribution!

lib/node_modules/@stdlib/complex/parse-float64/examples/index.js Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/complex/parse-float64/docs/types/index.d.ts Outdated Show resolved Hide resolved
@Planeshifter
Update lib/node_modules/@stdlib/complex/parse-float64/examples/index.js
074005e 
Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
@Planeshifter
Update lib/node_modules/@stdlib/complex/parse-float64/docs/types/inde…
429dd82 
…x.d.ts

Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
@Planeshifter Planeshifter merged commit 1154a11 into stdlib-js:develop Feb 28, 2024
7 checks passed
@Planeshifter Planeshifter removed the Needs Review A pull request which needs code review. label Mar 26, 2024
@Snehil-Shah Snehil-Shah mentioned this pull request Mar 29, 2024
Closed
6 tasks
@Snehil-Shah Snehil-Shah deleted the parse-float64 branch April 26, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stdlib-bot stdlib-bot stdlib-bot left review comments

@Planeshifter Planeshifter Planeshifter approved these changes

@Pranavchiku Pranavchiku Awaiting requested review from Pranavchiku

@kgryte kgryte Awaiting requested review from kgryte

Assignees
No one assigned
Labels
Feature Issue or pull request for adding a new feature. Needs Changes Pull request which needs changes before being merged. Utilities Issue or pull request concerning general utilities.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFC]: add @stdlib/complex/parse-float64
5 participants
@Snehil-Shah @kgryte @Planeshifter @stdlib-bot @Pranavchiku