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

Avoid target leakage and Improve Docs #15

Merged
merged 8 commits into from
Apr 20, 2023

Conversation

furkanmtorun
Copy link
Collaborator

@furkanmtorun furkanmtorun commented Apr 16, 2023

Hi Max,

# What's Done

  • Covered vast majority of the issues in OmicLearn v1.4 Updates #4
  • ONLY code change is on omiclearn/utils/ui_components.py. I tried to disable user to introduce target leakage
  • The rest: Improved docs by moving to sphinx-material theme (I thought it would be easy but took a lot of time and a lot of works, but looks now definitely better than current one)
  • Updated Docs and README
  • Removed old and unnecessary notebook and csv files

As for the Docs:

You need to install sphinx-material via pip.
I could not figure out how this works with ReadTheDocs but maybe you can let me know or handle it.

New SS:
Screenshot 2023-04-16 at 20 06 23

Screenshot 2023-04-16 at 20 06 34

Reproducibility Check

I run the following selections on both v1.3.1 on OmicLearn.com and v.1.4 on this branch and find both ZIP files below (they look same to me if I do not miss. It is better if we have 4 eyes on it)
Screenshot 2023-04-16 at 22 48 37

Copy link
Member

@straussmaximilian straussmaximilian left a comment

Choose a reason for hiding this comment

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

Hi,
I am not familiar with custom themes on RTD.org.
Built-in themes seem to work, e.g. https://rtd-sphinx-theme-sample-project.readthedocs.io/en/latest/

You could always host via GitHub Pages instead of RTD.org. We have done this in the past but there the issue was that some things didn't work as planned e.g. interactive display of notebooks and the PDF export was missing.

omiclearn/utils/ui_components.py Show resolved Hide resolved
@furkanmtorun
Copy link
Collaborator Author

furkanmtorun commented Apr 20, 2023

Hi @straussmaximilian ,

  • As for the documentation, normally, I was in favor of MKDocs and never used RTD. As stated, now, I put a lot of time here and we wrote this down in the paper. Therefore, I would like to go with RTD for this project.
    Could you please let me know how do you trigger RTD.org or login? Bcs, with GitHub signup, I can only create a new docs project instead of triggering/updating the current one.

  • As for target leakage, I understood your comment but actually, could not find a point where it is useful since the performance will be dream metrics 100% TP, TN etc. all the time for target leakages. So, I am eager to avoid such behaviour.

  • As for reproducibility, did you have a chance to check out the ZIP file outputs?

It would be nice if you can ping me about them when you have time :) Now, I will merge.

@furkanmtorun furkanmtorun merged commit 260791b into OmicLearn-v1.4 Apr 20, 2023
@furkanmtorun furkanmtorun deleted the improve-usability branch April 20, 2023 19:25
@straussmaximilian
Copy link
Member

Hi @furkanmtorun

  • Documentation: I added a Webhook via the Settings (Webhook)
  • Target Leakage: The interesting part is that it is not always dream metrics but should be training error and dependent by classifer. You can try it yourself by using a regularized classifier, e.g. the LogisticRegression (sklearn has regularization by default). Quick check on the AD dataset gives AUC of 0.85. - This would be the thing to show.
  • Quickly checked the zips: They look similar. Good job!

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

Successfully merging this pull request may close these issues.

None yet

2 participants