Questions regarding small improvements

[hannbus]

Here are the questions we could not discuss yesterday:

• What to write for the return value in the method head if the method returns a figure or a pandas dataframe? If I put in the type of the return value for example 'pandas.core.series.Series' I get a warning that pandas is not defined and pytest throws an error collecting the tests.
• For the scaffold analysis there is a naming problem: for now I used scaffold (+ terminal atoms), framework (- terminal atoms), and skeleton (only single bonds and C atoms). But the function is called 'draw_most_frequent_scaffolds' and in the function I used 'structure' as umbrella term. Are there any suggestions what words to use to make it understandable?
• Where to put the basic configuration of the logger?
• At some points I used a logging.warning, that should be alright?
• When there are no SDFiles in the given directory for the import there will be a logger warning, but would it be better to throw an error?

[JonasSchaub]

• What to write for the return value in the method head if the method returns a figure or a pandas dataframe? If I put in the type of the return value for example 'pandas.core.series.Series' I get a warning that pandas is not defined and pytest throws an error collecting the tests.

Since you import pandas as pd, maybe try pd.core.series.Series? And matplotlib.figure probably does not work because matplotlib itself is not imported?

• For the scaffold analysis there is a naming problem: for now I used scaffold (+ terminal atoms), framework (- terminal atoms), and skeleton (only single bonds and C atoms). But the function is called 'draw_most_frequent_scaffolds' and in the function I used 'structure' as umbrella term. Are there any suggestions what words to use to make it understandable?

I think the RDKit term for what you call skeleton is "generic scaffold", right? The term from the orginal Murcko and Bemis paper on molecular frameworks would be "archetype" or "graph framework". So it would be better is you chose one of these.
I actually think scaffold can be used as the umbrella term here.

• Where to put the basic configuration of the logger?

Something I also don't know. I would suggest placing it in the constructor. Or you can keep it in the "open class space" because a logger usually is a static object that is shared between every instance of a class. Then, it would actually be confusing to put it into the constructor that is tied to only one instance of the class.
I think for the logging functionality, there are a few things to discuss. Maybe, we open a separate thread for this?

• At some points I used a logging.warning, that should be alright?

That seems to be appropriate at the respective cases.

• When there are no SDFiles in the given directory for the import there will be a logger warning, but would it be better to throw an error?

I would vote for throwing an error. In this case, there is no reason to go any further with CIDER and it definitely indicates that the user made a mistake.

[hannbus]

Thanks for the answers!
I have another question: All methods that use the identifier list as input will fail when the identifiers are not calculated. Now automaticly a KeyError is raised. Is this fine or should it be an error raised from Cider indicating what to do?

[Kohulan]

Maybe you can handle the Error and report it back to the user. Rather than leaving it as a python error.

[hannbus]

Thanks for answering, I got this done.

[JonasSchaub]

Is there any other open question in this thread?

[hannbus]

No, all questions have been answered.