BlueBrain / nexus-forge

Building and Using Knowledge Graphs made easy
https://nexus-forge.readthedocs.io
GNU Lesser General Public License v3.0
38 stars 19 forks source link

Many methods default to `pretty=True`, and use `print()` #236

Open mgeplf opened 2 years ago

mgeplf commented 2 years ago

I believe that nexus-forge is mainly to be used as a library (correct me if I'm wrong), and thus I find the decision to print() things pecurliar. Normally, I would expect a library to remain as quiet as possible, possibly using the logging or warnings packages to give feedback in error conditions.

In this case, however, the following API calls appear to print by default:

KnowledgeGraphForge.prefixes
KnowledgeGraphForge.types
KnowledgeGraphForge.template
KnowledgeGraphForge.resolvers

Could we reopen this design decision?

MFSY commented 2 years ago

Hi @mgeplf,

At the beginning, the goal was more to use forge in the context of a Jupyter notebook. Hence the prints across the methods you highlighted. But we are happy to see it now used also as a lib. I know some of the team member found this pecurliar too.

Could we reopen this design decision?

Happy to discuss this with you. Currently, a client can always set 'pretty=False' or a different 'output' to get a result rather than printing. Are you suggesting to have it the other way around, ie. default to returning ? Or do you have other suggestion ?

@kplatis , @annakristinkaufmann , @eugeniashurko, @Nabil-AL : what are you thoughts on this?

mgeplf commented 2 years ago

the goal was more to use forge in the context of a Jupyter notebook.

Yup, fair enough, I'm just hoping we can revisit that decision; if we can't, I understand.

I wonder if we can rely more on jupyter to present the data. For instance, one can do:

from IPython.display import JSON
JSON(some_dict) # will display a dictionary nicely
JSON(some_json) # will display the json nicely

Having the user in the habit of printhing things when they want allows for more options, imo.

Are you suggesting to have it the other way around, ie. default to returning ?

Yes, I think that would be the minimal change.

I also think that: 1) 'pretty=True' should return the object, instead of only printing 2) using print() makes it very inflexible from a presentation standpoint; Jupyter has a whole host of options for rich presentation: https://ipython.readthedocs.io/en/stable/config/integrating.html#rich-display

mgeplf commented 2 years ago

Any updates on this? Does it seem like a good direction?

MFSY commented 2 years ago

Hi. @mgeplf ,

Any updates on this? Does it seem like a good direction?

Yes. Thanks for the input. I'll pick up this item next week.