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

g.citation: update regex to get date and author from manual #808

Merged
merged 9 commits into from
Oct 16, 2022

Conversation

ninsbl
Copy link
Member

@ninsbl ninsbl commented Oct 5, 2022

Replace #677 that was accidentally closed

@ninsbl ninsbl added the bug Something isn't working label Oct 5, 2022
@ninsbl
Copy link
Member Author

ninsbl commented Oct 6, 2022

OK to merge? Tests succeed as far as they can...

@ninsbl ninsbl requested a review from tmszi October 6, 2022 20:53
@tmszi
Copy link
Member

tmszi commented Oct 7, 2022

OK to merge? Tests succeed as far as they can...

I apologize, but since Wednesday evening I have a health problem that does not allow me to comfortably work on the computer. I should be fine within five days, maybe sooner. Please wait for my review. I'll look into it as soon as I can.

src/general/g.citation/g.citation.py Show resolved Hide resolved
src/general/g.citation/testsuite/test_g_citation.py Outdated Show resolved Hide resolved
src/general/g.citation/g.citation.py Outdated Show resolved Hide resolved
src/general/g.citation/g.citation.py Outdated Show resolved Hide resolved
src/general/g.citation/g.citation.py Show resolved Hide resolved
src/general/g.citation/testsuite/test_g_citation.py Outdated Show resolved Hide resolved
# TODO: raise or fatal? should be in library or module?
raise RuntimeError("The text does not contain date entry")
raise RuntimeError("Could not parse date from maual")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

manual

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea right now how shown to the user, but would prefer a complete sentence. :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing error messages is hard. I'm not sure what should be there. I think I understand why you don't like the original one, but the new one does not feel right either. I find this article informative, but still hard to implement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting and useful article. I tried to clarify why the error may have occurred...

@ninsbl
Copy link
Member Author

ninsbl commented Oct 14, 2022

@tmszi do you think this is ready to merge?

@tmszi
Copy link
Member

tmszi commented Oct 16, 2022

During testing, I found that the module output parameter does not work, it is not implemented. I have prepared a PR locally that fixes this bug and I will open it, when you merge this PR.

GRASS nc_spm_08_grass7/landsat:~ > g.citation v.surf.rst output=/tmp/result

GRASS nc_spm_08_grass7/landsat:~ > file /tmp/result
/tmp/result: cannot open `/tmp/result' (No such file or directory)

@ninsbl ninsbl merged commit de2dc8e into OSGeo:grass8 Oct 16, 2022
@ninsbl ninsbl deleted the g_citation_3rd branch October 16, 2022 10:42
cwhite911 pushed a commit to cwhite911/grass-addons that referenced this pull request Sep 19, 2023
* add testsuite

* capture author and date from manual

* adress code review

* adress code review

* fix spelling

* correct date extract doc/message

* black

* improve error message

* implement suggestions from review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants