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

replacing QtXmlPatterns because of missing implementation in some (higher) PyQt5 and up #13

Open
oldGreyEagle opened this issue Aug 28, 2024 · 9 comments

Comments

@oldGreyEagle
Copy link

This valuable plugin is broken for missing QtXmlPatterns on my Installation:

3.22.4-Białowieża
QGIS-Codezweig
[Release 3.22](https://github.com/qgis/QGIS/tree/release-3_22)
Qt-Version 5.15.3
Python-Version 3.10.3
GDAL-Version 3.4.1
PROJ-Version 8.2.1
EPSG-Registraturdatenbankversion v10.041 (2021-12-03)
GEOS-Version 3.10.2-CAPI-1.16.0
SQLite-Version 3.37.2
PostgreSQL-Client-Version 14.2 (Ubuntu 14.2-1)
SpatiaLite-Version 5.0.1
QWT-Version 6.1.4
QScintilla2-Version 2.11.6
BS-Version Ubuntu 22.04.4 LTS

As QtXmlPatterns is not always (no more) available (deprecation) I replaced it with lxml etree.
I can provide the code if you are interested. I'm not certain how to file a pull request for this.
Will be happy about a reply.

@ejn
Copy link
Contributor

ejn commented Aug 28, 2024

That sounds like a useful change - at the very latest then when QGIS changes over completely to Qt6 then this will certainly be an issue. So yes, definitely interested in your code!

Have you made your changes in a local Git repository / working copy or just directly edited the code outside of version control?

@oldGreyEagle
Copy link
Author

I have a local GIT repo containing the changes.

@ejn
Copy link
Contributor

ejn commented Aug 28, 2024

Perfect. Then you should be able to:

  1. Fork this repository on GitHub whilst you're logged in
  2. Add your fork as a new remote on your local repository
  3. Push your changes to the fork
  4. Open a pull request from your fork

Unfortunately pushing to Github has got a bit tricker now you've got to generate tokens rather than just using the username and password. A search engine of your choice should be able to help you with each step if you're not sure!

If it's too fiddly with the push then if you can get at least get your local repo published somewhere then the rest should be no problem.

@oldGreyEagle
Copy link
Author

oldGreyEagle commented Aug 28, 2024

I admit I did not originally use this repo to set up my local version but a zipped 0.8.1 I found. I just forked this repo but it seems I need to use the Makefile here. Unfortunately I run into errors doing so. As I need to install from zip to test, I added 3 files from my original sources (would normally be made I guess):

  • ui_inspireatomclient.py
  • ui_metadataclient.py
  • resources.py

With this it all works. I also added an option to pick ones output folder. Before I make a pull request, would it be better if you look into
https://github.com/oldGreyEagle/qgis-inspire-atom-client-plugin
first?
Maybe you can point me to how to get the make running properly. That would be safer. I guess the readme also needs an update since the plugin seems not in the official QGIS repo anymore.
Thanks for your time!

@oldGreyEagle
Copy link
Author

oldGreyEagle commented Aug 28, 2024

... fixed, see below ...
I just noticed, I have ruff set up to format my code. I'm really sorry, the diff got real ugly that way. The real change is in metadata_request_finished and taking out the MessageHandler that relied on the removed lib.
Picking the folder was added in an extra commit.
Sorry about forgetting to put ruff into a separate commit.

@oldGreyEagle
Copy link
Author

I just noticed, I have ruff set up to format my code. I'm really sorry, the diff got real ugly that way. The real change is in metadata_request_finished and taking out the MessageHandler that relied on the removed lib. Picking the folder was added in an extra commit. Sorry about forgetting to put ruff into a separate commit.

I decided I will fix the diff. This is too confusing.

@oldGreyEagle
Copy link
Author

oldGreyEagle commented Aug 29, 2024

I just noticed, I have ruff set up to format my code. I'm really sorry, the diff got real ugly that way. The real change is in metadata_request_finished and taking out the MessageHandler that relied on the removed lib. Picking the folder was added in an extra commit. Sorry about forgetting to put ruff into a separate commit.

I decided I will fix the diff. This is too confusing.

Done. You can now see everything in the corresponding commits without getting that noise from applying ruff. Should I make a pull request right away or will you be able to look into the make issue?

@ejn
Copy link
Contributor

ejn commented Aug 29, 2024

Thanks, and doubly so for fixing the diff :). I'll try and look into the build with make and testing this in the coming days (it could be a couple of weeks though before I find the time). As you recognised, those 3 files should be generated rather than being in the repo.

@oldGreyEagle
Copy link
Author

Ok, thank you. Looking forward to your findings!

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