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 bugs to run tutorial CLI, update python, matplotlib API #137

Merged
merged 11 commits into from
Oct 23, 2024
2 changes: 1 addition & 1 deletion doc/source/tutorial/abbreviations.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
.. |tar.gz| replace:: :source-release:`tar.gz`

.. Image substitutions
.. |images/extract_single_peak1.png| ge:: ../../examples/images/extract_single_peak1.png
.. |images/extract_single_peak1.png| image:: ../../examples/images/extract_single_peak1.png
sbillinge marked this conversation as resolved.
Show resolved Hide resolved
.. |images/extract_single_peak2.png| image:: ../../examples/images/extract_single_peak2.png
.. |images/extract_single_peak3.png| image:: ../../examples/images/extract_single_peak3.png
.. |images/parameter_summary1.png| image:: ../../examples/images/parameter_summary1.png
Expand Down
23 changes: 23 additions & 0 deletions news/entry-point-.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* <news item>

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* Configure entry point in pyproject.toml to run CLI commands

**Security:**

* <news item>
23 changes: 23 additions & 0 deletions news/fix-api-bug.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* <news item>

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* Update Python, matploblib API to run documentation CLI tutorials

**Security:**

* <news item>
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ template = "{tag}"
dev_template = "{tag}"
dirty_template = "{tag}"

[project.scripts]
srmise = "diffpy.srmise.applications.extract:main"

[tool.setuptools.packages.find]
where = ["src"] # list of folders that contain the packages (["."] by default)
include = ["*"] # package names should match these glob patterns (["*"] by default)
Expand Down
74 changes: 39 additions & 35 deletions src/diffpy/srmise/applications/extract.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,82 +419,85 @@ def main():
from diffpy.srmise.pdfpeakextraction import PDFPeakExtraction
from diffpy.srmise.srmiseerrors import SrMiseDataFormatError, SrMiseFileError

try:
options.peakfunction = eval("peaks." + options.peakfunction)
except Exception as err:
print(err)
print("Could not create peak function '%s'. Exiting." % options.peakfunction)
return

try:
options.modelevaluator = eval("modelevaluators." + options.modelevaluator)
except Exception as err:
print(err)
print("Could not find ModelEvaluator '%s'. Exiting." % options.modelevaluator)
return

if options.bcrystal is not None:
if options.peakfunction:
sbillinge marked this conversation as resolved.
Show resolved Hide resolved
try:
options.peakfunction = eval("peaks." + options.peakfunction)
except Exception as err:
print(err)
print("Could not create peak function '%s'. Exiting." % options.peakfunction)
Copy link
Contributor

Choose a reason for hiding this comment

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

These error messages could be improved by giving the user hints about how to fix them. Let's make an issue to make them more informative for the user, but don't worry about closing that issue in this release.

In general we need tests in srmise (please can you make an issue for that too?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

issue created

return

if options.modelevaluator:
try:
options.modelevaluator = eval("modelevaluators." + options.modelevaluator)
except Exception as err:
print(err)
print("Could not find ModelEvaluator '%s'. Exiting." % options.modelevaluator)
return

if options.bcrystal:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to use is not None since False is the default value.

from diffpy.srmise.baselines.polynomial import Polynomial

bl = Polynomial(degree=1)
options.baseline = parsepars(bl, [options.bcrystal, "0c"])
options.baseline.pars[0] = -4 * np.pi * options.baseline.pars[0]
elif options.bsrmise is not None:
elif options.bsrmise:
# use baseline from existing file
blext = PDFPeakExtraction()
blext.read(options.bsrmise)
options.baseline = blext.extracted.baseline
elif options.bpoly0 is not None:
elif options.bpoly0:
from diffpy.srmise.baselines.polynomial import Polynomial

bl = Polynomial(degree=0)
options.baseline = parsepars(bl, [options.bpoly0])
elif options.bpoly1 is not None:
elif options.bpoly1:
from diffpy.srmise.baselines.polynomial import Polynomial

bl = Polynomial(degree=1)
options.baseline = parsepars(bl, options.bpoly1)
elif options.bpoly2 is not None:
elif options.bpoly2:
from diffpy.srmise.baselines.polynomial import Polynomial

bl = Polynomial(degree=2)
options.baseline = parsepars(bl, options.bpoly2)
elif options.bseq is not None:
elif options.bseq:
from diffpy.srmise.baselines.fromsequence import FromSequence

bl = FromSequence(options.bseq)
options.baseline = bl.actualize([], "internal")
elif options.bspherical is not None:
elif options.bspherical:
from diffpy.srmise.baselines.nanospherical import NanoSpherical

bl = NanoSpherical()
options.baseline = parsepars(bl, options.bspherical)

try:
options.baseline = eval("baselines." + options.baseline)

except Exception as err:
print(err)
print("Could not create baseline '%s'. Exiting." % options.baseline)
return

filename = args[0]

if filename is not None:
if filename:
ext = PDFPeakExtraction()
try:
ext.read(filename)
except (SrMiseDataFormatError, SrMiseFileError, Exception):
ext.loadpdf(filename)

pdict = {}
if options.peakfunction is not None:
if options.peakfunction:
pdict["pf"] = [options.peakfunction]
if options.baseline is not None:
if options.baseline:
pdict["baseline"] = options.baseline
if options.cres is not None:
if options.cres:
pdict["cres"] = options.cres
if options.dg_mode is None:
if options.dg is not None:
if options.dg:
options.dg_mode = "absolute"
elif ext.dy is None:
options.dg_mode = "max-fraction"
Expand All @@ -510,17 +513,17 @@ def main():
pdict["effective_dy"] = options.dg * ext.y.ptp() * np.ones(len(ext.y))
elif options.dg_mode == "dG-fraction":
pdict["effective_dy"] = options.dg * ext.dy
if options.rng is not None:
if options.rng:
pdict["rng"] = list(options.rng)
if options.qmax is not None:
if options.qmax:
pdict["qmax"] = options.qmax if options.qmax == "automatic" else float(options.qmax)
if options.nyquist is not None:
if options.nyquist:
pdict["nyquist"] = options.nyquist
if options.supersample is not None:
if options.supersample:
pdict["supersample"] = options.supersample
if options.scale is not None:
if options.scale:
pdict["scale"] = options.scale
if options.modelevaluator is not None:
if options.modelevaluator:
pdict["error_method"] = options.modelevaluator

if options.liveplot:
Expand All @@ -532,23 +535,24 @@ def main():
cov = None
if options.performextraction:
cov = ext.extract()
print(cov)

if options.savefile is not None:
if options.savefile:
try:
ext.write(options.savefile)
except SrMiseFileError as err:
print(err)
print("Could not save result to '%s'." % options.savefile)

if options.pwafile is not None:
if options.pwafile:
try:
ext.writepwa(options.pwafile)
except SrMiseFileError as err:
print(err)
print("Could not save pwa summary to '%s'." % options.pwafile)

print(ext)
if cov is not None:
if cov:
print(cov)

if options.plot:
Expand Down
2 changes: 1 addition & 1 deletion src/diffpy/srmise/applications/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ def on_draw(event):
invisiblelabel.set_visible(True)
visiblelabel = labeldict[fig]
bbox = invisiblelabel.get_window_extent(invisiblelabel._renderer)
bbox = bbox.inverse_transformed(ax_main.transAxes)
bbox = bbox.transformed(ax_main.transAxes.inverted())
sbillinge marked this conversation as resolved.
Show resolved Hide resolved
bbox = bbox.get_points()
xpos = np.mean(np.transpose(bbox)[0])

Expand Down
9 changes: 7 additions & 2 deletions src/diffpy/srmise/basefunction.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ def __init__(
The class (not instance) which implements caching of BaseFunction
evaluations.
"""

self.parameterdict = parameterdict
self.npars = len(self.parameterdict)

Expand All @@ -114,9 +115,13 @@ def __init__(
if not isinstance(p, str):
emsg = "Argument parameterdict's keys must be strings."
raise ValueError(emsg)
vals = self.parameterdict.values()

# Convert values to list and sort
vals = list(self.parameterdict.values())
vals.sort()
if vals != range(self.npars):

# Check if the sorted values match the sequence from 0 to npars-1
if vals != list(range(self.npars)):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bug fix - compare list against list in the latest Python

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's dump an issue to handle this better too. It strikes me as a brittle design. But don't fix this in this release. Something for a new group member.

emsg = (
"Argument parameterdict's values must uniquely specify "
+ "the index of each parameter defined by its keys."
Expand Down
5 changes: 4 additions & 1 deletion src/diffpy/srmise/pdfdataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,10 @@ def read(self, filename):
self
"""
try:
self.readStr(open(filename, "rb").read())
# Open the file in binary mode, read it, and decode to convert bytes to string
with open(filename, "rb") as file:
file_content = file.read().decode("utf-8")
sbillinge marked this conversation as resolved.
Show resolved Hide resolved
self.readStr(file_content)
except PDFDataFormatError as err:
basename = os.path.basename(filename)
emsg = ("Could not open '%s' due to unsupported file format " + "or corrupted data. [%s]") % (
Expand Down