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

Checklist for 0.1 release #5

Closed
7 tasks done
ablaom opened this issue Sep 28, 2021 · 9 comments
Closed
7 tasks done

Checklist for 0.1 release #5

ablaom opened this issue Sep 28, 2021 · 9 comments

Comments

@ablaom
Copy link
Member

ablaom commented Sep 28, 2021

These are my suggestions:

  • Check coverage. I'd be happy with > 80%
  • Re-organize the transformer and tests into separate files to facilitate future extension
  • Add documentation. I'd be happy with dumping the transformer docstring into the README.md for now. But if someone wants to do something more elaborate.... . My suggestion would be to keep the documentation local, ie, not add to MLJ central docs. We can put a link there.
  • Immigration of scitype stuff Migrate text-specific scitype definitions into this package #7 @ablaom to do
  • Check we have complete set of [compat] entries including julia version
  • CI passing
  • Create a new master branch and merge a "For a 0.1 release" PR dev -> master

@pazzo83 @storopoli Feel free to add to this.

@storopoli
Copy link
Collaborator

Maybe a simple "hello world" example with the documentation?

@pazzo83
Copy link
Collaborator

pazzo83 commented Oct 5, 2021

Agree on all this - what's the best way to set up coverage? I think I saw some output in the CI where our coverage was above 80%.
In terms of reorganizing the transformer/tests - do you mean putting what we've defined into a separate file (and the corresponding tests as well)?

@ablaom
Copy link
Member Author

ablaom commented Oct 5, 2021

Coverage is setup. The readme icon is not showing the coverage for reasons I do not understand, but if I click on it, it takes you to the detailed analysis (which is > 80%). Maybe if we make a few more PR's it will fix itself. As I say, 80% is okay with me but it might be worth just scanning the analysis to see what is missing.

In terms of reorganizing the transformer/tests - do you mean putting what we've defined into a separate file (and the corresponding tests as well)?

Yes. Just move your code to src/tfidf_transformer.jl and your tests to test/tifid_transformer.jl and add includes to bring those in. Make sense?

@ablaom
Copy link
Member Author

ablaom commented Oct 19, 2021

Okay, revisiting coverage, I realised that the new scitype tests are orphaned. 🙄 Working on a fix now.

edit Done.

@ablaom
Copy link
Member Author

ablaom commented Oct 19, 2021

@pazzo83 You happy for me to release this now?

@pazzo83
Copy link
Collaborator

pazzo83 commented Oct 19, 2021

Absolutely! Thank you for all of your help with this!!

@ablaom
Copy link
Member Author

ablaom commented Oct 19, 2021

No worries. Thanks for your patience.

@ablaom ablaom closed this as completed Oct 19, 2021
@ablaom
Copy link
Member Author

ablaom commented Oct 19, 2021

@storopoli
Copy link
Collaborator

Great!!! Happy to help you guys!

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

No branches or pull requests

3 participants