common.py across notebooks should be shared.

[andorsk]

Import issue with common.py where it needs to be shared across notebooks, but based on feedback of structure from animikh, it needs to be in parent folder, which casued some import issues. Look into this.

[andorsk]

https://github.com/animikhroy/rk_toolkit_pipeline_diagrams/tree/main/02_notebooks/common

[andorsk]

Note: merging will be a little weird multiple people work on this together, particularly since we are possibly editing the notebook, be aware probably better to coordinate prior to work.

[ghost]

There's a helpers.py file in the other notebook folders that are being needed for common.py to work well, so either I should move the helpers.py file from the rk_gw_mma or rk_general_applications to common folder or relatively import them (which may cause issues when importing and should be addressed ).

[andorsk]

@Ashxyz998 common shouldn't require helpers.py. If it does, that's a mistake. Common should be used across notebooks. Helpers should be for a specific notebook.

[ghost]

Yeah figured out where I was wrong. Will look into it. Also have to update the function names for the mahalonobis and jaccard in it to magnitudinal_dist_function and topological_distance_function respectively as those are the new names for these functions

[andorsk]

@Ashxyz998 I don't think that's correct tbh. I think mahalonobis function is correctly named. But what there could be is a specific signature for each method that does designate whether its' magnitudinal or topological.

A distance method generally follows the following interface:

dist(x,y) -> d <- and there may be many types of distances functions.

What the mag or top distance metric is suggesting is that the distance function be applied for a specific type of measurement. So maybe mahalonobis becomes a class, with a method inside it, referencing type. Or, and probably better, just make a function

psuedo-code:

def get_signature(f) -> str:
     switch f.__name__:
          case mahalonobis:
                  return "mag"
         case jaccard:
                  return "top"

[ghost]

where should this method be added. Should it be in the Graph class in models.py or in the distance.py file. Also should I change the code in graph class which are currently referring to these methods.;

[andorsk]

@Ashxyz998 I added an issue: https://github.com/andorsk/rk_toolkit/issues/13. The names need to be reverted back to the original names.

The signature can be in the graph.py file.

[ghost]

@andorsk the following line of code resolves the common.py import issue. I've added the code here as my edited notebook document is filled with errors and doesn't work properly, so commiting that output would mess up with the notebooks in the repo. So could you add these lines and resolve it?

import sys sys.path.append("../common") #Run this the first time(everytime the notebook is reopened) to import common library

[ghost]

I've also noticed that the common.py files were different in common folder and in the rk_gw_mma. I think the one in rk_gw_mma folder is more recent but would request you to confirm it when you have time and place the one to be used in the common folder

[ghost]

import sys sys.path.append("../common") #Run this the first time(everytime the notebook is reopened/initiliazed) to import common library

This code needs to be added at the start of the notebooks which use the common package. This will temporarily add the path for importing

[ghost]

This issue can be closed after confirming it works

[andorsk]

thanks @Ashxyz998 . Good callout on the merge. Yea...that makes sense. I can do that. Thanks for adding the fix. Will close when it's tested.

[andorsk]

@Ashxyz998 great thought. It worked. Closing. Thanks.