search

OpenMS / pyopenms-docs

pyOpenMS readthedocs documentation, additional utilities, addons, scripts, and examples.
https://pyopenms.readthedocs.io
Other
42 stars 52 forks source link

Explicitly import pyOpenMS #278

Closed Kastakin closed 1 year ago

Kastakin commented 2 years ago

Star imports are a bad practice and should be avoided in the examples proposed in the documentation.

While learning the OpenMS framework I usually imported the package with import pyopenms as ms, maybe it's possible to decide on a two letter identifier to import the package.

I can work on the issue if it's considered worth by everyone and not just me!

jpfeuffer commented 2 years ago

Totally agree. And I like ms.

timosachsenberg commented 2 years ago

good point! agree

tjeerdijk commented 2 years ago

Same here. It has always annoyed me that the star import does not make explicit which functions or methods from OpenMS are being used in the examples.

tapaswenipathak commented 2 years ago

Should @greengypsy or I takeover this issue?

timosachsenberg commented 2 years ago

@Kastakin did you already move forward here? Just checking because I would like to prevent duplicated work if @tapaswenipathak or @greengypsy take over.

Kastakin commented 2 years ago

Sorry for the late reply, I'm having a busy month due to some university workshops. I think I'll be able to work on the task from the 12th of June forward.

If you intend to start working on it maybe I can join in later if we coordinate properly!

timosachsenberg commented 2 years ago

I leave this to @tapaswenipathak or @greengypsy to decide. Not super high priority but nice to have.

tapaswenipathak commented 2 years ago

On Wed, Jun 1, 2022 at 9:29 PM Lorenzo Castellino @.***> wrote:

Sorry for the late reply, I'm having a busy month with some university workshops. I think I'll be able to work on the task from the 12th of June forward.

If you intend to start working on it maybe I can join in later if we coordinate properly!

Hi Lorenzo! we'll start on 12th June, see you at https://discord.com/invite/swkxs72CHB.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

JeeH-K commented 1 year ago

@timosachsenberg Could you help me understand this issue a bit closer? We just want to change all the example codes in the pyopenms-docs repository as import pyopenms as ms from from pyopenms import *?

jpfeuffer commented 1 year ago

Yes, either this and then use ms.MSExperiment() etc. Or just import what is needed (from pyopenms import MSExperiment, FileHandler, ...)

tjeerdijk commented 1 year ago

My preference would be the first option to make explicit what is part of pyOpenMS. Explicitly:

import pyopenms as poms poms.MSExperiment()

(Other abbreviations than poms are fine too, I like poms)

On 8. May 2023, at 16:46, Julianus Pfeuffer @.**@.>> wrote:

Yes, either this and then use oms.MSExperiment() etc. Or just import what is needed (from pyopenms import MSExperiment, FileHandler, ...)

— Reply to this email directly, view it on GitHubhttps://github.com/OpenMS/pyopenms-docs/issues/278#issuecomment-1538482145, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGKDZED5TI3HDWV5VHA53KDXFEBKVANCNFSM5V3HVNAA. You are receiving this because you commented.Message ID: @.***>

JeeH-K commented 1 year ago

Thanks for the clarifications! My vote for abbreviation is on poms ;) Since I'm quite occupied this week, I'll start working on it in the next sprint. Hope that's okay.

timosachsenberg commented 1 year ago

haha that is funny - I personally used oms so far ( I guess because it sounds more like a mantra (and poms like the small town Boms ;) )