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

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

Open
mgeplf opened this issue Feb 14, 2022 · 4 comments
Open

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

mgeplf opened this issue Feb 14, 2022 · 4 comments

Comments

@mgeplf
Copy link

mgeplf commented Feb 14, 2022

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
Copy link
Collaborator

MFSY commented Feb 14, 2022

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
Copy link
Author

mgeplf commented Feb 15, 2022

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
Copy link
Author

mgeplf commented Mar 10, 2022

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

@MFSY
Copy link
Collaborator

MFSY commented Mar 17, 2022

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.

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

No branches or pull requests

2 participants