Skip to content

Move doc from comments to meta (aircrfta)#407

Open
tmigot wants to merge 2 commits intomainfrom
move-docstring-to-metadata
Open

Move doc from comments to meta (aircrfta)#407
tmigot wants to merge 2 commits intomainfrom
move-docstring-to-metadata

Conversation

@tmigot
Copy link
Copy Markdown
Member

@tmigot tmigot commented Feb 27, 2026

No description provided.

@tmigot
Copy link
Copy Markdown
Member Author

tmigot commented Feb 27, 2026

@arnavk23 @dpo

@tmigot
Copy link
Copy Markdown
Member Author

tmigot commented Mar 29, 2026

@dpo @arnavk23 I updated the PR to take into account the suggestions. For the specific example illustrated here, I didn't find a DOI. Ideally, the URL field would use the DOI link.
If you guys agree I can then apply to all problems.

@tmigot tmigot marked this pull request as ready for review March 29, 2026 17:30
@tmigot tmigot requested review from Copilot and dpo March 29, 2026 17:30
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR relocates the aircrfta problem’s provenance/licensing documentation from the PureJuMP implementation file into the corresponding Meta dictionary.

Changes:

  • Removed the large header comment block from src/PureJuMP/aircrfta.jl.
  • Added :url, :notes, :origin_notes, and :reference entries to src/Meta/aircrfta.jl.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/PureJuMP/aircrfta.jl Removes the embedded provenance/license header from the problem implementation file.
src/Meta/aircrfta.jl Stores the removed provenance/license information (plus URL/reference) in the problem’s metadata dict.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 1 to 3
export aircrfta

function aircrfta(; n::Int = default_nvar, kwargs...)
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

The upstream license/permission text that was previously embedded in this source file has been removed. Since the permission grant explicitly requires the copyright/permission notice to appear in all copies, keeping the notice only in a separate Meta file may not satisfy that requirement (and is inconsistent with other PureJuMP problem files that retain this header). Consider restoring a minimal copyright/permission header in this file (or otherwise ensuring the notice is included with this source when distributed).

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +23
:url => "https://www.osti.gov/biblio/6449249",
:notes => raw"""
Converted in Julia from https://github.com/mpf/Optimization-Test-Problems

AMPL Model by Hande Y. Benson
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

The newly added :url, :notes, :origin_notes, and :reference keys are not part of the aggregated OptimizationProblems.meta DataFrame (which only copies a fixed list of keys). If these fields are meant to be discoverable via the standard metadata interface, consider extending cols_names/types and the DataFrame population logic (or adding a documented accessor) so users can query them without reaching into aircrfta_meta directly.

Copilot uses AI. Check for mistakes.
@tmigot
Copy link
Copy Markdown
Member Author

tmigot commented Mar 29, 2026

Connected to #319 and #324

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants