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

Please avoid including libraries #3

Open
pcav opened this issue Sep 24, 2019 · 9 comments
Open

Please avoid including libraries #3

pcav opened this issue Sep 24, 2019 · 9 comments

Comments

@pcav
Copy link

pcav commented Sep 24, 2019

This is convenient, but in a wider perspective can create issues. If the libs are not available in the standard installers, please consider helping adding them to it.
Thanks for understanding.

@matheusfillipe
Copy link
Owner

I am only using PIL as a fallback for raster computation and values extraction. PIL is not available in the standard installer and I don't really think it should be... I've packaged both windows and linux versions but the program can probably run without it anyway. PIL isn't necessary and I could remove it.

In the other hand PyQtGraph is quite fundamental for this project, hence is used for displaying and editing cross sections and other things that aren't quite the goal of qgis itself. We have been testing this on Linux and Windows 10 without problems with those libraries.

@pcav
Copy link
Author

pcav commented Sep 26, 2019

I think PyQtGraph can be added to thee standard installer for win at least (unsure about OSX).
Could you please point me out to PIL?
Thanks.

@matheusfillipe
Copy link
Owner

I am using a slightly modified version of pyqtgraph that I did for this project btw. PIL here is actually it's fork PIllow: https://pypi.org/project/Pillow/
Can't this project really be added the way it is now, at least as experimental? We are willing to test with many students and having it in the repo would make this easier for them.

@pcav
Copy link
Author

pcav commented Oct 2, 2019

Hi, thanks for the reply. I'm still not convinced this is the best approach. Forking to me is evil ;)
you are welcome to discuss this in the public mailing list, as it has general relevance.
Thanks.

@matheusfillipe
Copy link
Owner

By looking at: https://plugins.qgis.org/publish/ There's nothing about not being allowed forked libraries which their license would allow to.So I thought that it was better to eliminate dependencies and just get this working easier for the users perspective.
I will take your advice and discuss this in the public mailing list. Thanks.

@luipir
Copy link

luipir commented Oct 10, 2019

IMHO can be approved, there is no binary inside and all is pure python (no binary at all)! I would only suggest to add pythonpath to use that library only when necessary to avoid to interfere with distributed pillow of pyqtchart (if will be available)
I can't see reasons to avoid merge, there are already many library packaged in plugins to manage dependency. Usually including official weels instead of replicating all the libs, but I can't see problem to customise (for any reason) a third party library.

@NyakudyaA
Copy link

NyakudyaA commented Oct 25, 2019

@pcav considering the above comment. Should I approve this plugin?

@pcav
Copy link
Author

pcav commented Oct 25, 2019

I don't agree with this, as it does not help improving the core libs, duplicates info, and create a babelization of software (consider we are handling > on thousand plugins, we could easily end up with hundreds of different versions).
This is what we have always asked until now, and it proved successful, I don't see a strong reason to change.
I understand people outside the day do day work of plugin management can underrate these problems.

@matheusfillipe
Copy link
Owner

@pcav I agree with you that avoiding duplication is the right way to go. I have removed the PIL library as I said it could be done. Still I can't remove PyqtGraph since it is a fork and the tendency is that more things will be modified on it.
This software deals with specific 3D data by editing cross sections of it, given some set of rules, which I couldn't do at all by overriding classes from pyqtgraph... so was just easier to modify it. I don't think It was written anywhere that libraries aren't allowed so if that's so, better that be added before someone else planning to add plugins to the qgis repo does like me.
I still think that in cases where it is a forked library or even when it isn't a library that comes in a standard qgis install, for the users side is better that the library comes embed on the plugin since it gives an easier installation. That surely not the best option for developers or for improving qgis, but that being said.

The truth is, since this is a university project that has been developed during 3 years by scholarship and I will be probably the last working on it, which I hope I am not, and it is the first open source alternative to Bentley's Topograph (proprietary), or Civil 3D (for road design), that I see, even though is still small compared to that, I am willing to make it more popular by having it on the main repo. My orienter already said that his first plan was to freeze on qgis 3.10 with this plugin installed, kinda forking qgis, for the next students until another scholarship comes to improve it and if that developers is willing to test a new qgis version. I don't agree but is the best solution if the plugin isn't easy to install in any qgis version. I did the work of porting it from qgis 2.8 to qgis 3.x and already reported the bugs i found on the APi, so I get when you talk about the importance of improving the core libs

That's all I have to say. Pyqtgraph can't really be removed. Sorry for the poor english.

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

4 participants