search

mlpack / models

models built with mlpack
https://models.mlpack.org/docs
BSD 3-Clause "New" or "Revised" License
35 stars 41 forks source link

Adding documentation for dataloader. #9

Closed kartikdutt18 closed 4 years ago

kartikdutt18 commented 4 years ago

I have updated readme a little bit to show use of data loaders and pre-processor functions. I have left utility functions out for now. Let me know if I should include them. One other is I have copied the Contribution.md from mlpack repo. I think it makes sense to have a wiki page for data loaders, utility functions and pre-processors. Kindly let me know if the wiki page idea makes sense. Thanks a lot. Looking forward to your suggestions. Regards.

kartikdutt18 commented 4 years ago

Hey @birm, @zoq, When you get a chance could you please take a look at the documentation that I added in the wiki here. If this makes sense I can add the same for rest of the functions.

KimSangYeon-DGU commented 4 years ago

Awesome!

I have left utility functions out for now. Let me know if I should include them.

@kartikdutt18 Can you let me know what the utility functions are?

kartikdutt18 commented 4 years ago

@kartikdutt18 Can you let me know what the utility functions are?

Hey @KimSangYeon-DGU,

Other than the dataloader we also added :

  1. PreProcess Class that preprocess data.

  2. Utility class which has the following functions :

    1. DownloadFile : Downloads Files
    2. CheckCRC32 : Performs Checksum
    3. Remove : Removes files from directory
    4. GetCRC32 : Generates CRC32 for a file.

Kindly let me know what you think about the dataloader documentation if that makes sense, I can add similar documentation for the rest of functions. Thanks a lot.

KimSangYeon-DGU commented 4 years ago

Hi @kartikdutt18, in my opinion, it seems to be better for us to keep README.md concise focusing on the main points like you've done :)

Regarding functions in utils, we already give explanations in detail, so I think it's okay not to write such the info for the additional utility functions. However, if you think we should handle that as well, please go ahead :) (Maybe, it's a little hard to keep updating those docs in the future..., so automating approach from comments would be a workaround..., but that's out of this PR)

kartikdutt18 commented 4 years ago

Hi @kartikdutt18, in my opinion, it seems to be better for us to keep README.md concise focusing on the main points like you've done :)

Great, I'll just add some more properties regarding the dataloader then.

(Maybe, it's a little hard to keep updating those docs in the future..., so automating approach from comments would be a workaround..., but that's out of this PR)

That would be really great.

KimSangYeon-DGU commented 4 years ago

Great, I'll just add some more properties regarding the dataloader then.

Ok, thanks!

kartikdutt18 commented 4 years ago

I have updated the wiki with a few corrections. Kindly let me know what you think. This is ready from my side. If the wiki is good to, I think I can port it to this repo as well.

KimSangYeon-DGU commented 4 years ago

Wiki is also nice :)

kartikdutt18 commented 4 years ago

Great, Let me know when to port it to the main models repo. Thanks a lot for the reviews.

KimSangYeon-DGU commented 4 years ago

@kartikdutt18 I think after merging this PR, it's good time to add wiki :)

kartikdutt18 commented 4 years ago

Great. Will add it then.

KimSangYeon-DGU commented 4 years ago

@kartikdutt18 Can you look through Style Checks?

kartikdutt18 commented 4 years ago

Done, The style failure was related to files that were stored here earlier. I have done a rebase to fix that :)

KimSangYeon-DGU commented 4 years ago

@kartikdutt18 Looks good to me. Thanks!

@birm Please merge this, if you're fine with the change :)

kartikdutt18 commented 4 years ago

Awesome, Thanks a lot @KimSangYeon-DGU, @birm, @zoq for all the reviews and suggestions. I'll copy over the wiki now.