qgis / qgis4.0_api

Tracker for QGIS 4.0 API related issues and developer discussion
3 stars 1 forks source link

Drop Qgs class prefix in classes #17

Closed wonder-sk closed 7 years ago

wonder-sk commented 8 years ago

The fact that everything needs to be written as QgsFeature, QgsVectorLayer etc makes PyQGIS look alien to people from Python world. Such prefixes are legacy of C world that does not support namespaces. Python has concept of namespaces, C++ has concept of namespaces.

Option 1: Drop Qgs in C++ and Python. Use namespaces in the C++ code (e.g. qgis::core::Feature) and have PyQGIS equivalents qgis.core.Feature. Of course in C++ we would normally just make use of using namespace and thus write Feature instead of full name.

Option 2: Drop Qgs in Python only. C++ code would stay as is, Python code would have classes like Feature. For the conservative ones, we could provide aliases so they would still be able to write QgsFeature

nyalldawson commented 7 years ago

After much deliberation I'm also in favour of this!

timlinux commented 7 years ago

Which one are you in favour of @nyalldawson (option 1 or 2)?

We could also use one bug namespace for all QGIS classes couldn't we (so qgis::feature)?

My main concern is that now every single line of legacy QGIS Python code will be broken (even if the fix is a relatively easy s/Qgs//g), the chance of having a common code base providing code for both QGIS 2.x and 3.x will probably be zero. Python users could import as to alias the old class names as the new (from qgis.core import QgsFeature as Feature when using 2.x api) but that is also going to be not so much fun.

@nyalldawson could add some more notes explaining why you think it is a good idea and if it outweighs the grumbles we will inevitably get from Python authors who have existing code to maintain?

nyalldawson commented 7 years ago

Option 1.

I actually honestly feel that there's no possible way that plugins can be written to handle both 2.x and 3.x simultaneously. I think the fork-and-target-3.0 only approach is going to prove to be the only workable approach.

But that's beside the point. I originally thought we could just start doing this with new classes.... but then sometime in the future we'll want to change the existing classes anyway (for consistency, predictability, etc). And then why not just do this now while the api is changing so radically anyway?

3nids commented 7 years ago

I didn't follow very closely, is there a custom 2to3 for plugins? If not, we really should bring one. Is there something like this for the grant?

If there is a nice tool proposed to migrate, I'm also in favor of option 1. Not that I am afraid of doing some simple regex myself ;) but I'm just very afraid of the reaction.

m-kuhn commented 7 years ago

Yeah, the Qgs-prefix looks really odd when developing python. I think most people just got used to it and no longer mind after some initial time using it.

Besides the doubts about porting and backwards compatibility, this will also render many snippets in the net like for example stackoverflow outdated. These code snippets serve a lot of beginners as kickstarters.

FYI, the QFieldSync plugin works with QGIS 2.18 and QGIS 2.99 with the relatively simple qgis2compat api wrapper until now. Many things "just work" without the wrapper.

I am not sure if we make life easier or harder for developers with this change.

alexbruy commented 7 years ago

+1 for dropping Qgs prefix.

NathanW2 commented 7 years ago

I'm in the same boat. I don't like it but the wide scale impact makes me thing -1 for a drop just because of the pain from it.

m-kuhn commented 7 years ago

I prefer the approach with the python aliases. We should just ship them out of the box. C++ can be updated or not, I don't expect much trouble there apart from some pull requests that need rebasing. Newly introduced classes will only be available prefix-less.

Then we update our own sample code all over the place to the new API and let the world start teaching people the new prefix-less way for a couple of years.

We finish the move with dropping the aliases for QGIS 4.0 when nobody uses the ugly old-style no longer anyway.

NathanW2 commented 7 years ago

So example

qgis.gui.MapCanvas
qgis.gui.QgsMapCanvas

both work

but something new like my user profile stuff:

qgis.core.ProfileManager

Does this mean we should drop the prefix in C++ and just alias only in Python to not cause wide spread damage?

3nids commented 7 years ago

+1 to this proposition, it is a really good compromise!

NathanW2 commented 7 years ago

+1 I'm good with this idea.

Do we also rename the files qgsmapcanvas.cpp mapcanvas.cpp?

3nids commented 7 years ago

yes, rename files too.

one question would be for direct Qt inherited objects like QgsSpinBox, QgsDockWidget. For those it might make sense to keep a prefix, but maybe Qgis ?

m-kuhn commented 7 years ago

Most of our classes inherit Q-prefix classes (QObject somewhere in the end). I assume you are referring to widgets. Do they need special treatment?

What I would think is also very nice are these things

dakcarto commented 7 years ago

+1, though I do think the larger community may not see the benefit, thereby complain.

If the 2to3 tool can manage most (if not all) conversion cases, that should alleviate most detractors. Updating PRs doesn't seem too problematic.

g-sherman commented 7 years ago

This hurts me—so bad…

On Mar 14, 2017, at 7:31 PM, Nyall Dawson notifications@github.com wrote:

After much deliberation I'm also in favour of this!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/qgis/qgis3.0_api/issues/17#issuecomment-286631241, or mute the thread https://github.com/notifications/unsubscribe-auth/AAeKyuRaVSepRHYr6LqmYo-8-v4GL0xTks5rl1uUgaJpZM4GtPxu.

--------------------- Gary Sherman Founder, QGIS Project -Open Source GIS Books: http://locatepress.com http://locatepress.com/ -Geospatial Consulting & Hosting: http://geoapt.com http://geoapt.com/ "We work virtually everywhere" ---------------------

3nids commented 7 years ago

I did a quick try with a script. see https://github.com/qgis/QGIS/pull/4267

3nids commented 7 years ago

before going any further, can I consider there is a consensus here?

m-kuhn commented 7 years ago

This hurts me—so bad…

Very sorry to hear, I very well remember your blog post almost 3 years ago which made my day and still is a fun-fact that I enjoy to tell people once in a while. Sorry for not bringing this up earlier in this discussion.

NathanW2 commented 7 years ago

I'm still on the fence with this because it makes me semi-uncomfortable. Here are some reasons:

Pros for dropping:

Cons for dropping:

Pros for keeping:

Cons for keeping:

*** On this point, just because we can break API doesn't mean we should. We risk killing a lot of plugins coming back if there is too much work to do. We also risk user base loss if the move process isn't easy. So generally I'm not sure I'm keen on change just for the sake of change. Unless there is a good solid reason.

strk commented 7 years ago

On Thu, Mar 16, 2017 at 03:31:51AM -0700, Nathan Woodrow wrote:

search for quick ref and QgsFeature is better then just Feature)

This is a great reason not to change the name (my 2 cents)

3nids commented 7 years ago

Another drawback is the loss of quick git history due to files being renamed

NathanW2 commented 7 years ago

Honestly I'm just not feeling the love for dropping the prefix at all at this current time.

nyalldawson commented 7 years ago

Yeah... Maybe not a good move. Sorry for the noise!

NathanW2 commented 7 years ago

Not noise. It's why we talk it through in the end.