TutteInstitute / vectorizers

Vectorizers for a range of different data types
BSD 3-Clause "New" or "Revised" License
97 stars 23 forks source link

Example/documentation of InformationWeightTransformer #134

Closed kalebruscitti closed 3 months ago

kalebruscitti commented 4 months ago

I've added a notebook to the docs on the InformationWeightTransformer, using the recipe data from the tutorials repo.

Someone more knowledgeable than me should review it for correctness before it gets merged.

kalebruscitti commented 4 months ago

You can preview my doc at https://timc-vectorizers.readthedocs.io/en/latest/information_weight_transform.html

codecov-commenter commented 4 months ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.20%. Comparing base (2086155) to head (a5a3645). Report is 1 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #134 +/- ## ======================================= Coverage 86.20% 86.20% ======================================= Files 34 34 Lines 5174 5177 +3 ======================================= + Hits 4460 4463 +3 Misses 714 714 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kalebruscitti commented 4 months ago

@hamelin Colin read over this, at least thoroughly enough to correct a typo, so you should not feel a need to review anymore it if you don't wish to.

hamelin commented 3 months ago

Also, please examine what is this warning that prevents the unit tests from succeeding. I strongly dislike not having a sweet sweet green state for unit tests. :-D If the warning sounds complicated to you and you feel uncomfortable fixing its source yourself, please capture it along with the circumstance around its occurrence, and report it as an issue I can fix once I come back from vacay.

kalebruscitti commented 3 months ago

It appears to me that the tests failing is older than my changes: image

hamelin commented 3 months ago

@kalebruscitti why did you not merge the branch and closed it instead?

kalebruscitti commented 3 months ago

@hamelin Uh I guess I messed up at using GitHub LOL

I thought that it was all finished so I deleted my fork. I still have a local copy of the code though so I can easily bring it back if needed... let me know what the best way to fix it is.

hamelin commented 3 months ago

Please start a new PR. We will tie it to this discussion and merge it.