diff --git a/corelib/src/libs/SireIO/gro87.cpp b/corelib/src/libs/SireIO/gro87.cpp index 7f1c84b88..adb44d753 100644 --- a/corelib/src/libs/SireIO/gro87.cpp +++ b/corelib/src/libs/SireIO/gro87.cpp @@ -693,54 +693,75 @@ Frame Gro87::getFrame(int i) const { i = SireID::Index(i).map(this->nFrames()); - return SireMol::Frame(); -} + QVector frame_coords; + if (not this->coords.isEmpty()) + { + frame_coords = this->coords[i]; + } -/** Return the Gro87 object that contains only the information for the ith - frame. This allows you to extract and create a system for the ith frame - from a trajectory */ -Gro87 Gro87::operator[](int i) const -{ - i = Index(i).map(this->nFrames()); + QVector frame_vels; - if (nFrames() == 1) - return *this; + if (not this->vels.isEmpty()) + { + const auto velocities = this->vels[i]; + const int nats = frame_vels.count(); - Gro87 ret(*this); + frame_vels = QVector(nats); - if (not coords.isEmpty()) - { - ret.coords = {coords[i]}; - } + auto velocities_data = frame_vels.data(); + const auto vels_data = velocities.constData(); - if (not vels.isEmpty()) - { - ret.vels = {vels[i]}; - } + // velocity is Angstroms per 1/20.455 ps + const auto vel_unit = (1.0 / 20.455) * angstrom / picosecond; - if (not current_time.isEmpty()) - { - ret.current_time = {current_time[i]}; + for (int i = 0; i < nats; ++i) + { + const Vector &vel = vels_data[i]; + velocities_data[i] = Velocity3D(vel.x() * vel_unit, vel.y() * vel_unit, vel.z() * vel_unit); + } } - if (not box_v1.isEmpty()) + // Convert the box vectors into a Space object. + SpacePtr space; + if (not this->box_v1.isEmpty() and not this->box_v2.isEmpty() and not this->box_v3.isEmpty()) { - ret.box_v1 = {box_v1[i]}; - } + Vector v1 = this->box_v1[i]; + Vector v2 = this->box_v2[i]; + Vector v3 = this->box_v3[i]; - if (not box_v2.isEmpty()) + if (v1.y() == 0 and v1.z() == 0 and v2.x() == 0 and v2.z() == 0 and v3.x() == 0 and v3.y() == 0) + { + // PeridicBox + space = SpacePtr(new PeriodicBox(Vector(v1.x(), v2.y(), v3.z()))); + } + else + { + // TriclinicBox + space = SpacePtr(new TriclinicBox(v1, v2, v3)); + } + } + else { - ret.box_v2 = {box_v2[i]}; + // no box information + space = SpacePtr(new Cartesian()); } - if (not box_v3.isEmpty()) + Time frame_time; + if (not this->current_time.isEmpty()) { - ret.box_v3 = {box_v3[i]}; + frame_time = this->current_time[i] * SireUnits::picosecond; } - ret.assertSane(); + return SireMol::Frame(frame_coords, frame_vels, space.read(), frame_time); +} - return ret; +/** Return the Gro87 object that contains only the information for the ith + frame. This allows you to extract and create a system for the ith frame + from a trajectory */ +Gro87 Gro87::operator[](int i) const +{ + throw SireError::unsupported( + QObject::tr("Gro87::operator[] is not yet implemented!"), CODELOC); } /** Return the parser that has been constructed by reading in the passed diff --git a/corelib/src/libs/SireIO/moleculeparser.cpp b/corelib/src/libs/SireIO/moleculeparser.cpp index 305be0f87..b9c89a8dc 100644 --- a/corelib/src/libs/SireIO/moleculeparser.cpp +++ b/corelib/src/libs/SireIO/moleculeparser.cpp @@ -1669,7 +1669,35 @@ QStringList MoleculeParser::writeToFile(const QString &filename) const QStringList written_files; - createDirectoryForFile(filename); + // Check whether list of trajectory frames have been specified via the map. + // If so, then the name will be prefixed with "frames_names:" + QStringList frame_names; + if (filename.contains("frames_names:")) + { + // Split the part of the string following "frames_names:" by commas to get + // individual frame names. + const QString frames_names_str = filename.section("frames_names:", 1); + frame_names = frames_names_str.split(",", Qt::SkipEmptyParts); + + // Convert to absolute paths. + for (auto &frame_name : frame_names) + { + frame_name = QFileInfo(frame_name.trimmed()).absoluteFilePath(); + } + + if (frame_names.size() != frames_to_write.size()) + { + throw SireError::program_bug( + QObject::tr("The number of frame names provided (%1) does not match the number of frames to write (%2).") + .arg(frame_names.size()) + .arg(frames_to_write.size()), + CODELOC); + } + } + else + { + createDirectoryForFile(filename); + } if (this->writingTrajectory() and this->isFrame()) { @@ -1694,51 +1722,61 @@ QStringList MoleculeParser::writeToFile(const QString &filename) const const int padding = QString::number(largest_frame).length(); - // in this case, we are going to create a directory named after the - // filename, into which all the frames will be written - auto fileinfo = QFileInfo(filename); + QDir framedir; + QFileInfo fileinfo; + bool write_to_frame_dir = false; - if (fileinfo.exists()) + if (frame_names.isEmpty()) { - // by default, we support overwriting of files - so remove this if it is a file - if (fileinfo.isDir()) - { - _check_and_remove_frame_dir(fileinfo); + fileinfo = QFileInfo(filename); + write_to_frame_dir = true; - // rebuild so it is not cached - fileinfo = QFileInfo(filename); + // in this case, we are going to create a directory named after the + // filename, into which all the frames will be written + fileinfo = QFileInfo(filename); - if (fileinfo.exists()) - throw SireError::file_error(QObject::tr( - "Could not write the trajectory for file '%1' as there " - "is already a directory with this name that contains " - "files that don't appear to have been created by sire.") - .arg(filename), - CODELOC); - } - else + if (fileinfo.exists()) { - auto dir = fileinfo.absoluteDir(); - - if (not dir.remove(fileinfo.fileName())) - throw SireError::file_error(QObject::tr( - "Could not write the trajectory for file '%1' as " - "we don't have permission to remove the existing " - "file with this name.") - .arg(filename), - CODELOC); + // by default, we support overwriting of files - so remove this if it is a file + if (fileinfo.isDir()) + { + _check_and_remove_frame_dir(fileinfo); + + // rebuild so it is not cached + fileinfo = QFileInfo(filename); + + if (fileinfo.exists()) + throw SireError::file_error(QObject::tr( + "Could not write the trajectory for file '%1' as there " + "is already a directory with this name that contains " + "files that don't appear to have been created by sire.") + .arg(filename), + CODELOC); + } + else + { + auto dir = fileinfo.absoluteDir(); + + if (not dir.remove(fileinfo.fileName())) + throw SireError::file_error(QObject::tr( + "Could not write the trajectory for file '%1' as " + "we don't have permission to remove the existing " + "file with this name.") + .arg(filename), + CODELOC); + } } - } - QDir framedir(fileinfo.absoluteFilePath()); + framedir = QDir(fileinfo.absoluteFilePath()); - if (not framedir.mkpath(".")) - throw SireError::file_error(QObject::tr( - "Could not create the directory into which to write the " - "trajectory for '%1'. Check that there is enough space " - "and you have the correct permissions.") - .arg(framedir.absolutePath()), - CODELOC); + if (not framedir.mkpath(".")) + throw SireError::file_error(QObject::tr( + "Could not create the directory into which to write the " + "trajectory for '%1'. Check that there is enough space " + "and you have the correct permissions.") + .arg(framedir.absolutePath()), + CODELOC); + } const auto suffix = fileinfo.completeSuffix(); @@ -1765,13 +1803,26 @@ QStringList MoleculeParser::writeToFile(const QString &filename) const // construct a copy of this parser for this frame auto parser = this->construct(thread_s, m); - // now write it to the file, numbered by the frame number and time - QString frame_filename = framedir.filePath( - "frame_" + - QString::number(i).rightJustified(padding, '0') + - "_" + - QString::number(time).replace(".", "-") + - "." + suffix); + QString frame_filename; + + if (write_to_frame_dir) + { + // now write it to the file, numbered by the frame number and time + QDir framedir(fileinfo.absoluteFilePath()); + + frame_filename = framedir.filePath( + "frame_" + + QString::number(i).rightJustified(padding, '0') + + "_" + + QString::number(time).replace(".", "-") + + "." + suffix); + } + else + { + // write to the specified frame name + frame_filename = frame_names[i]; + createDirectoryForFile(frame_filename); + } parser.read().writeToFile(frame_filename); @@ -1792,13 +1843,26 @@ QStringList MoleculeParser::writeToFile(const QString &filename) const // construct a copy of this parser for this frame auto parser = this->construct(s, m); - // now write it to the file, numbered by the frame number - QString frame_filename = framedir.filePath( - "frame_" + - QString::number(i).rightJustified(padding, '0') + - "_" + - QString::number(time).replace(".", "-") + - "." + suffix); + QString frame_filename; + + if (write_to_frame_dir) + { + // now write it to the file, numbered by the frame number + QDir framedir(fileinfo.absoluteFilePath()); + + frame_filename = framedir.filePath( + "frame_" + + QString::number(i).rightJustified(padding, '0') + + "_" + + QString::number(time).replace(".", "-") + + "." + suffix); + } + else + { + // write to the specified frame name + frame_filename = frame_names[i]; + createDirectoryForFile(frame_filename); + } parser.read().writeToFile(frame_filename); @@ -1810,7 +1874,14 @@ QStringList MoleculeParser::writeToFile(const QString &filename) const // only return the directory name, as we can handle // reading all the frames contained therein - written_files.append(framedir.absolutePath()); + if (write_to_frame_dir) + { + written_files.append(framedir.absolutePath()); + } + else + { + written_files.append(frame_names); + } } else { @@ -2452,6 +2523,38 @@ QStringList MoleculeParser::write(const System &system, const QString &filename, } } + // Check for a frame_names property in the map, which is used to control the naming of + // specific frames when writing trajectories. + if (map.specified("frame_names")) + { + const auto frame_names_property = map["frame_names"]; + + QStringList frame_names; + + if (frame_names_property.hasSource()) + { + frame_names = frame_names_property.source().split(","); + } + else + { + try + { + frame_names = frame_names_property.value().asA().toString().split(","); + } + catch (...) + { + throw SireError::incompatible_error( + QObject::tr("The 'frame_names' property must be a StringProperty containing " + "a comma-separated list of filenames to use for each frame when " + "writing trajectories."), + CODELOC); + } + } + + // add the special prefix to the first filename + filenames[0] = "frames_names:" + frame_names.join(","); + } + // now we have a list of filenames and associated formats, actually // write the files return ::pvt_write(system, filenames, fileformats, map); diff --git a/doc/source/changelog.rst b/doc/source/changelog.rst index 3fea1f740..2d225b595 100644 --- a/doc/source/changelog.rst +++ b/doc/source/changelog.rst @@ -33,6 +33,11 @@ organisation on `GitHub `__. * Added support for saving crash reports during Sire dynamics runs. +* Fix missing ``Sire::IO::Gro87::getFrame`` implementation to allow creation of + trajectory frames from GROMACS coordinate file data. + +* Add support for writing trajectory frames to user-defined file names. + `2025.2.0 `__ - October 2025 -------------------------------------------------------------------------------------------- diff --git a/doc/source/tutorial/part01/06_trajectories.rst b/doc/source/tutorial/part01/06_trajectories.rst index 88aa7c578..9a89aeb3e 100644 --- a/doc/source/tutorial/part01/06_trajectories.rst +++ b/doc/source/tutorial/part01/06_trajectories.rst @@ -157,5 +157,17 @@ System( name=output num_molecules=1 num_residues=3 num_atoms=22 ) 1000 The individual frame files are named using the frame number plus the time in picoseconds for that frame. +Rather than writing to a frame directory, it is also possible to use specific +names for each frame file by passing a list of filenames to :func:`sire.save`, +e.g.: + +>>> f = sr.save(mols[0].trajectory()[:2], ["cat.pdb", "dog.pdb"]) +>>> print(f) +['cat.pdb', 'dog.pdb'] + +When specifying frame names, the number of names provided must match the +number of frames being saved. In addition, the frames must have a common +file format (e.g. all PDB, or all GRO etc). + :mod:`sire` has extensive support for trajectories. To learn more, check out the :doc:`detailed guide <../../cheatsheet/trajectory>`. diff --git a/src/sire/_load.py b/src/sire/_load.py index 0943ea9a0..27891d69d 100644 --- a/src/sire/_load.py +++ b/src/sire/_load.py @@ -524,7 +524,7 @@ def save_to_string( def save( molecules, - filename: str, + filename: _Union[str, _List[str]], format: _Union[str, _List[str]] = None, save_velocities: bool = True, show_warnings=True, @@ -552,7 +552,9 @@ def save( filename (str): The name of the file to which to write the file. Extensions will be automatically added if they are needed to match - the formats of the file (or files) that are written. + the formats of the file (or files) that are written. If + 'molecules' is a TrajectoryIterator, then this can also be + list of filenames, one for each frame of the trajectory. format (str or list(str)): The format (or formats) that should be used to write the @@ -628,7 +630,11 @@ def save( if not os.path.exists(directory): os.makedirs(directory) - filename = os.path.join(directory, filename) + if isinstance(filename, str): + filename = os.path.join(directory, filename) + elif isinstance(filename, (list, tuple)): + for i in range(0, len(filename)): + filename[i] = os.path.join(directory, filename[i]) if format is not None: if type(format) is str: @@ -637,6 +643,37 @@ def save( map.set("fileformat", ",".join(format)) if hasattr(molecules, "_is_trajectory_iterator"): + # The user has passed a list of filenames - one for each frame. + if isinstance(filename, (list, tuple)): + # Make sure the number of filenames matches the number of frames. + if len(filename) != molecules.num_frames(): + raise ValueError( + f"When saving a trajectory, if you pass a list of " + f"filenames, there must be one for each frame. " + f"You passed {len(filename)} filenames, but the " + f"trajectory has {molecules.num_frames()} frames." + ) + + # Make sure the filenames have consistent extensions. + exts = set() + for fname in filename: + import os + + exts.add(os.path.splitext(fname)[1]) + if len(exts) != 1: + raise ValueError( + "When saving a trajectory, if you pass a list of " + "filenames, they must all have the same file " + "extension." + ) + + # Set the 'frame_names' property in the map to the list of filenames. + map.set("frame_names", ",".join(filename)) + + # Set the filename to be the first filename in the list. This + # argument is redundant now, but MoleculeParser.save requires it. + filename = filename[0] + # Doing it this way rather that using type(molecules) # as type(molecules) randomly fails, and because # this way is more pythonic diff --git a/tests/io/test_trajectories.py b/tests/io/test_trajectories.py index 558e3652d..64d5b7983 100644 --- a/tests/io/test_trajectories.py +++ b/tests/io/test_trajectories.py @@ -128,3 +128,32 @@ def test_trajectories(tmpdir, ala_traj): assert_space_equal(m.property("space"), m2.property("space"), precision) assert_coords_equal(m[0], m2[0], precision) + + +def test_frame_names(tmpdir, ala_traj): + mols = ala_traj.clone() + + d = tmpdir.mkdir("test_frame_names") + + # First, write two trajectory frames to named Gro87 files. + sr.save(mols.trajectory()[0:2], [d.join("cat.gro"), d.join("dog.gro")]) + + # Make sure the output files exist. + cat_path = d.join("cat.gro") + dog_path = d.join("dog.gro") + assert cat_path.check() + assert dog_path.check() + + # Try writing to mixed formats. This should raise a ValueError. + with pytest.raises(ValueError): + sr.save( + mols.trajectory()[0:2], + [d.join("cat.gro"), d.join("dog.rst7")], + ) + + # Try writing with mismatched number of frames and filenames. + with pytest.raises(ValueError): + sr.save( + mols.trajectory()[0:3], + [d.join("cat.gro"), d.join("dog.gro")], + )