The Wayback Machine - https://web.archive.org/web/20201018182443/https://github.com/BrainJS/brain.js/issues/551
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

Migrate to TypeScript #551

Open
mubaidr opened this issue May 17, 2020 · 94 comments
Open

Migrate to TypeScript #551

mubaidr opened this issue May 17, 2020 · 94 comments

Comments

@mubaidr
Copy link
Contributor

@mubaidr mubaidr commented May 17, 2020

We plan to gradually migrate brain.js to TypeScript, code base is pretty large, so we would love your help! 💪

How to contribute?

  • Convert a file from .js to .ts
  • Add types, fix all type errors.
  • Submit a PR! 🎉

Here you can find a guide on how to contribute.

Want to convert something, let us know in the comment and go ahead! 😎

To avoid duplicate work please comment on which part you want to work on (as long as nobody else is working on it) so we can mark it as taken.

Reach out to us!
Feel free to reach if you have questions or need help getting started. You can leave comments here or you can tag me in your PR if you need any help or you're not sure about something!

You can also get in touch on our Gitter & Slack.

Happy Coding! 🤟


UPDATE:

Wohoooo!!! 🎉

All files inside src directory are migrated to typescript. (Except few ones which are already taken up and being worked on), though we are still looking on improvements to types in these files and removing any types from the source. You are welcome to contribute. 😊

__tests__ directory has still some files left that needs migration to typescript, so feel free to pick em up! 🍭

@mubaidr mubaidr pinned this issue May 17, 2020
@mubaidr mubaidr mentioned this issue Jun 15, 2020
0 of 10 tasks complete
@yashshah1
Copy link
Contributor

@yashshah1 yashshah1 commented Jul 25, 2020

I'd like to start working on this, I have some experience with Ts, can you tell me which module is up for grabs?

@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Jul 25, 2020

@yashshah1 You are welcome to contribute, please pick any module or portion for conversion and mention it here, so I can mark it in-progress (to avoid any duplication effort).

@nabeelvalley
Copy link
Contributor

@nabeelvalley nabeelvalley commented Aug 5, 2020

I'd like to work on this too, any recommendations on where to start?

@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Aug 5, 2020

@nabeelvalley here is the short guide, super simple to start. Please pick any module or portion for conversion and mention it here, so I can mark it in-progress (to avoid any duplication effort).

Here you can find a guide on how to contribute.

@nabeelvalley
Copy link
Contributor

@nabeelvalley nabeelvalley commented Aug 6, 2020

@mubaidr I'm working from the utilities up, to avoid having untyped dependencies

The tests seem to be failing when I convert the file to JS because it looks like Jest is running against the TS files instead of the compiled JS - has the TS Compile been configured?

Looks like the tests are also currently failing:

Test Suites: 1 failed, 5 passed, 6 of 64 total
Tests:       9 failed, 114 passed, 123 total
Snapshots:   0 total
Time:        206.723 s

It appears that the soft-max tests are receiving a Float32Array instead of a plain array - should I look where the Float32 is coming from or should I update the unit tests to expect a Floar32Array instead?

The data returned looks exactly the same, it's just that the object types are different so the deep equality is failing:

Example for one of the tests below:

  ● SoftMax › .compare2D › can run on a simple matrix

    assert.deepEqual(received, expected)

    Expected value to deeply equal to:
      [[-0, 2], [3, 4]]
    Received:
      [[-0, 2], [3, 4]]

    Difference:

    - Expected
    + Received

      Array [
    -   Array [
    +   Float32Array [
          -0,
          2,
        ],
    -   Array [
    +   Float32Array [
          3,
          4,
        ],
      ]
@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Aug 9, 2020

Yes, you can update test to expect Float32Array.

In the mean-time some tests might still fail, because they have not yet been updated recently. You can can continue working and make sure build process is successful and conversion does not cause increase in no. of failed tests.

@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Aug 28, 2020

Hi! I want to help. If I convert one file, do I need to convert related files too or something? Or just a single file?

@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Aug 28, 2020

You don't need to updated all the related files, just go file by file and make sure build is successful.

@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Aug 28, 2020

Utilities is taken, isnt it?

@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Aug 28, 2020

Activation functions seem like a good place to start, may I work on it?

@nabeelvalley
Copy link
Contributor

@nabeelvalley nabeelvalley commented Aug 28, 2020

Utilities is taken, isnt it?

Hi @HarshKhandeparkar, I haven't had a chance to work on this you're welcome to take utilities if you want, just note that needs to be updated above

@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Aug 28, 2020

Np @nabeelvalley. I think I'll work on activation functions :)

@HarshKhandeparkar HarshKhandeparkar mentioned this issue Aug 28, 2020
3 of 12 tasks complete
@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Aug 29, 2020

@nabeelvalley I think I managed to fix the jest error you were facing in #582. You may copy paste my changes to jest.config.json or wait for the PR to be merged :)

@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Aug 29, 2020

estimator/ has a single file. Looks like a nice 🎯.

@HarshKhandeparkar HarshKhandeparkar mentioned this issue Aug 29, 2020
3 of 12 tasks complete
@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Aug 30, 2020

I think I'll snipe some of the utilities next, if you don't mind @nabeelvalley.

@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Aug 30, 2020

You would have probably started in alphabetical order(I am assuming), so I am going to start from the bottom.

@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Sep 1, 2020

Going to snipe utilities/values* tonight :)

@robertleeplummerjr
Copy link
Contributor

@robertleeplummerjr robertleeplummerjr commented Sep 1, 2020

Huzzah!

@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Sep 1, 2020

Build systems seems to be broken, I am looking into this issue. 🧯

@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Sep 1, 2020

Can I continue or should I wait?

@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Sep 1, 2020

You should continue your contributions! I will try to fix it asap.

@robertleeplummerjr
Copy link
Contributor

@robertleeplummerjr robertleeplummerjr commented Oct 10, 2020

Can someone pickup src/recurrent/matrix/copy.js?

@HarshKhandeparkar
Copy link
Contributor

@HarshKhandeparkar HarshKhandeparkar commented Oct 10, 2020

It's already ts ¯\_(ツ)_/¯

@robertleeplummerjr
Copy link
Contributor

@robertleeplummerjr robertleeplummerjr commented Oct 10, 2020

Ah, I was looking at the export, not extension. I'll take care of converting to named export.

@LuanSilveiraSouza
Copy link
Contributor

@LuanSilveiraSouza LuanSilveiraSouza commented Oct 12, 2020

Hello! I want to contribute, can I convert tests/utilities/layer-size.js to typescript?

@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Oct 12, 2020

@LuanSilveiraSouza sure, go ahead!

@Nelias Nelias mentioned this issue Oct 12, 2020
5 of 11 tasks complete
@rohya8
Copy link

@rohya8 rohya8 commented Oct 12, 2020

I'd like to start working on this, can you tell me which module i can start with?

@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Oct 12, 2020

@rohya8 Please select any file(s) from __tests__ directory and just mention it here.

@AgentEnder
Copy link
Contributor

@AgentEnder AgentEnder commented Oct 12, 2020

mubaidr added a commit that referenced this issue Oct 13, 2020
convert __tests__/recurrent/unit.js => ts #551
@dhairyagada
Copy link

@dhairyagada dhairyagada commented Oct 14, 2020

I'll contribute to /__tests__/utilities/data-formatter.js!

@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Oct 14, 2020

@dhairyagada /__tests__/utilities/ is fully migrated to typescript as per this pull request: https://github.com/BrainJS/brain.js/pull/630/files

You can pick other files please.

@dhairyagada
Copy link

@dhairyagada dhairyagada commented Oct 14, 2020

@mubaidr Can I know which one's are remaining?

@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Oct 14, 2020

All other js files inside __tests__ are available. You can start with __tests__/neural-network/options.js

@dhairyagada
Copy link

@dhairyagada dhairyagada commented Oct 14, 2020

@mubaidr Thanks! I'll start with it 👍

@dhairyagada
Copy link

@dhairyagada dhairyagada commented Oct 14, 2020

@mubaidr Has src\neural-network.js already been converted to Typescript already? Because that is required for __tests__/neural-network/options.js

@robertleeplummerjr
Copy link
Contributor

@robertleeplummerjr robertleeplummerjr commented Oct 14, 2020

I have a locally converted tests/utilities/data-formatter.js

@LuanSilveiraSouza
Copy link
Contributor

@LuanSilveiraSouza LuanSilveiraSouza commented Oct 16, 2020

Hello again! I will convert /__tests__/neural-network/test.js to ts

@Satyam-kumar-yadav
Copy link

@Satyam-kumar-yadav Satyam-kumar-yadav commented Oct 16, 2020

Hey, I would like to convert /tests/neural-network/to-function.js to ts

@mubaidr
Copy link
Contributor Author

@mubaidr mubaidr commented Oct 16, 2020

@dhairyagada No, its in progress. But I don't think you would need it.
@LuanSilveiraSouza @Satyam-kumar-yadav Sure, go ahead!

@geekyrajshri
Copy link

@geekyrajshri geekyrajshri commented Oct 18, 2020

@mubaidr
Let me know if I can pick any of test files for converting to ts.

@dhairyagada dhairyagada mentioned this issue Oct 18, 2020
4 of 11 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.