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

fix(createpackages): avoid creating invalid escape characters #1055

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

mwtoews
Copy link
Contributor

@mwtoews mwtoews commented Feb 11, 2021

This tidies up a few DeprecationWarning messages from docstrings, and expand LaTeX commands like \mf to MODFLOW 6.

Flake8's W605 can be removed.

@codecov
Copy link

codecov bot commented Feb 11, 2021

Codecov Report

Merging #1055 (b2ada75) into develop (943ace4) will increase coverage by 1.075%.
The diff coverage is 73.333%.

@@              Coverage Diff              @@
##           develop     #1055       +/-   ##
=============================================
+ Coverage   71.769%   72.845%   +1.075%     
=============================================
  Files          225       221        -4     
  Lines        51895     51119      -776     
=============================================
- Hits         37245     37238        -7     
+ Misses       14650     13881      -769     
Impacted Files Coverage Δ
flopy/mf6/data/mfstructure.py 64.210% <0.000%> (-0.227%) ⬇️
flopy/mf6/modflow/mfgwf.py 55.555% <ø> (ø)
flopy/mf6/modflow/mfgwfgwf.py 55.172% <ø> (ø)
flopy/mf6/modflow/mfgwflak.py 56.790% <ø> (ø)
flopy/mf6/modflow/mfgwfmaw.py 54.794% <ø> (ø)
flopy/mf6/modflow/mfgwfnam.py 62.962% <ø> (ø)
flopy/mf6/modflow/mfgwfnpf.py 57.575% <ø> (ø)
flopy/mf6/modflow/mfgwfsfr.py 55.128% <ø> (ø)
flopy/mf6/modflow/mfgwfsto.py 64.516% <ø> (ø)
flopy/mf6/modflow/mfgwfuzf.py 54.666% <ø> (ø)
... and 11 more

@christianlangevin
Copy link

Hey @mwtoews, just wanted to give you a heads up that these modflow 6 classes that you modified are generated automatically from definition files that describe the package contents. So making changes to the docstrings won't have a long-lasting effect. We've tried to note this at the top of these files with:

# DO NOT MODIFY THIS FILE DIRECTLY.  THIS FILE MUST BE CREATED BY
# mf6/utils/createpackages.py

Agreed that it would be good to get this fixed, but it will require a different approach.

@mwtoews
Copy link
Contributor Author

mwtoews commented Feb 12, 2021

Déjà vu #700

@mwtoews mwtoews force-pushed the invalid-escape-sequences branch from 8732ec5 to 77fed19 Compare February 14, 2021 09:57
@mwtoews mwtoews changed the title fix(DeprecationWarning): resolve invalid escape sequences fix(createpackages): avoid creating invalid escape characters Feb 14, 2021
@mwtoews mwtoews force-pushed the invalid-escape-sequences branch from 77fed19 to e590edf Compare February 14, 2021 10:00
@mwtoews
Copy link
Contributor Author

mwtoews commented Feb 14, 2021

@langevin-usgs I tracked down the main bug to remove escape characters, which is why this PR has been rewritten

Some of the math is overkill, e.g. < and > only need a math environment in latex, not in a Python docstring. I've also added a few other changes, which may warrant further discussion. For instance, \citep{authoryear} is replaced with a Harvard-like (authoryear), which is slightly better (should citations without references be permitted?). As for snippets liketable~\ref{table:ftype}, is there a better translation than table ref{table:ftype}?

@christianlangevin
Copy link

Hey @mwtoews, I don't see how the change you made in this PR removes the :math: string. Am I missing something? Are there changes required to createpackages.py that are not included here?

And for table~\ref{table:ftype} maybe we should just say mf6io.pdf which is where that table is located.

* Fix latex escape characters in description processing with math
* Replace latex characters to plaintext for Python
* Replace several other latex items involving \citep and \ref
  that are specific to mfio.pdf
@mwtoews mwtoews force-pushed the invalid-escape-sequences branch from e590edf to 7cb3f46 Compare February 16, 2021 00:28
@mwtoews
Copy link
Contributor Author

mwtoews commented Feb 16, 2021

@langevin-usgs only :math: was removed for < and >. This was not a bug, but I thought it was unnecessary for Python. Only in latex is $<$ and $>$ necessary. Often in the same paragraphs a regular = is used. E.g. here, search near "the horizontal width" there is a math > with a non-math =.

As for \citep and \ref, take another look as some of these have some hard-coded replacements which I hope are correct. I've also just noticed that ` for single quote should be replaced with '.

@jdhughes-dev jdhughes-dev merged commit 8a2cffb into modflowpy:develop Feb 18, 2021
@mwtoews mwtoews deleted the invalid-escape-sequences branch March 21, 2021 20:48
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

Successfully merging this pull request may close these issues.

3 participants