Skip to content

feat : add complex/base/parse#1385

Merged
Planeshifter merged 12 commits into
stdlib-js:developfrom
marsian83:develop
Feb 26, 2024
Merged

feat : add complex/base/parse#1385
Planeshifter merged 12 commits into
stdlib-js:developfrom
marsian83:develop

Conversation

@marsian83

Copy link
Copy Markdown
Contributor

Description

This PR tackles the parsing as discussed here

This implements a parser which will convert a complex number represented as a string to a complex like object

This can be used as a base for @stdlib/complex/assert/is-complex and @stdlib/complex/parse-32 and @stdlib/complex/parse-64.
Detailed discussion as to why this approach may be chosen can be found #1332 & #1370

Related Issues

#1332
#1333
#1370

Checklist

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


@stdlib-js/reviewers

@marsian83 marsian83 changed the title feat : add @stdlib/assert/is-nonnegative-finite feat : add @stdlib/complex/base/parse Feb 25, 2024
@kgryte kgryte added Feature Issue or pull request for adding a new feature. Needs Review A pull request which needs code review. Utilities Issue or pull request concerning general utilities. labels Feb 25, 2024
@kgryte kgryte changed the title feat : add @stdlib/complex/base/parse feat : add complex/base/parse Feb 25, 2024
@kgryte

kgryte commented Feb 25, 2024

Copy link
Copy Markdown
Member

@marsian83 Looks like linting is failing. Mind resolving those lint errors?

@marsian83

Copy link
Copy Markdown
Contributor Author

@marsian83 Looks like linting is failing. Mind resolving those lint errors?

While linting this line is behaving weirdly
image

It requires me to have no spaces in JSDOC type definitions but also requires me to have spaces after the curly bracket opening and before it closing. how do I resolve this conflicting requirement

@marsian83

marsian83 commented Feb 26, 2024

Copy link
Copy Markdown
Contributor Author
image

These three are proving to be quite problematic to me, the first one, I do not understand.
The second is the one I discussed above.
The third one is asking me to require Number which is an inbuilt JS module?

Comment thread lib/node_modules/@stdlib/complex/base/parse/lib/main.js Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/lib/main.js Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/lib/main.js
Comment thread lib/node_modules/@stdlib/complex/base/parse/lib/main.js Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/lib/main.js Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/lib/main.js Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/lib/index.js Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/lib/index.js Outdated
Co-authored-by: Athan <kgryte@gmail.com>
Signed-off-by: Spandan Barve <114365550+marsian83@users.noreply.github.com>
@marsian83

Copy link
Copy Markdown
Contributor Author

I have noted and understood the changes you made and also commited and signed them off
I am still confused about the linter asking me to require 'Number' though

@marsian83

Copy link
Copy Markdown
Contributor Author

There was a slight lint error in the license headers. I have fixed and pushed changes

marsian83 and others added 3 commits February 26, 2024 09:51
Parses -> Parse
Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
Comment thread lib/node_modules/@stdlib/complex/base/parse/docs/types/test.ts Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/docs/repl.txt Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/docs/repl.txt Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/examples/index.js Outdated
Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
@Planeshifter

Copy link
Copy Markdown
Member

@marsian83 Thanks for making the requested changes and working on this new package in the first place! Made some further small changes to align with project conventions and will merge once CI passes. 🚀

Comment thread lib/node_modules/@stdlib/complex/base/parse/examples/index.js Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/examples/index.js
Comment thread lib/node_modules/@stdlib/complex/base/parse/examples/index.js Outdated
Comment thread lib/node_modules/@stdlib/complex/base/parse/examples/index.js
Comment thread lib/node_modules/@stdlib/complex/base/parse/examples/index.js
Comment thread lib/node_modules/@stdlib/complex/base/parse/examples/index.js Outdated
Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
@marsian83

Copy link
Copy Markdown
Contributor Author

@Planeshifter Thanks 😄 for bringing to my attention the conventions I missed, I will keep these in mind from next time on now that you have pointed these out.

Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
@Planeshifter Planeshifter merged commit 4c6159f into stdlib-js:develop Feb 26, 2024
@Planeshifter Planeshifter removed the Needs Review A pull request which needs code review. label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature Issue or pull request for adding a new feature. Utilities Issue or pull request concerning general utilities.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants