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

Remove nuisance variables #2

Merged
merged 70 commits into from
Dec 4, 2023
Merged

Remove nuisance variables #2

merged 70 commits into from
Dec 4, 2023

Conversation

pnavada
Copy link
Collaborator

@pnavada pnavada commented Sep 28, 2023

No description provided.

@pnavada pnavada requested a review from cthoyt September 28, 2023 12:32
@pnavada pnavada self-assigned this Sep 28, 2023
@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (816fdd4) 3.58% compared to head (e3c0cfe) 41.83%.

Files Patch % Lines
src/eliater/discover_latent_nodes.py 97.29% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            main       #2       +/-   ##
==========================================
+ Coverage   3.58%   41.83%   +38.24%     
==========================================
  Files          7       11        +4     
  Lines        251      306       +55     
  Branches      32       40        +8     
==========================================
+ Hits           9      128      +119     
+ Misses       242      176       -66     
- Partials       0        2        +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@cthoyt cthoyt left a comment

Choose a reason for hiding this comment

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

I added several inline TODO's that need to be addressed. I also set up the CI tests to fail until these TODOs addressed

@cthoyt cthoyt mentioned this pull request Dec 4, 2023
@cthoyt cthoyt changed the title Adding logic and tests to discover and mark latent nodes Remove nuisance variables Dec 4, 2023

.. code-block:: python

data = pd.read_csv(
Copy link
Member

Choose a reason for hiding this comment

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

data is missing from this PR

.. code-block:: python

data = pd.read_csv(
"../eliater/src/eliater/data/sachs_discretized_2bin.csv",
Copy link
Member

Choose a reason for hiding this comment

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

data is missing from this PR

@cthoyt
Copy link
Member

cthoyt commented Dec 4, 2023

I cleaned up the documentation in the following ways:

  1. Removed stub text for loading data that has not been committed to this branch. I added fixmes for these
  2. Recapitulated previous FIXMEs about needing more careful documentation about the example graphs.
  3. Replaced the incorrect images in the nuisance node removal tutorial. @srtaheri you can be more careful about this by actually running the code that you write
  4. Rewrite the nuisance node removal tutorial text - it's much more simple now

Remember @srtaheri @pnavada the point of all of this is to make documentation that a reader can actually follow and learn from

@cthoyt
Copy link
Member

cthoyt commented Dec 4, 2023

I'm going to merge this now despite there being some TODO's left

@cthoyt cthoyt merged commit 0b0c13d into main Dec 4, 2023
8 checks passed
@cthoyt cthoyt deleted the simplify branch December 4, 2023 10:44
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.

3 participants