Skip to content

Commit

Permalink
fix: improve peer review reminders rubric
Browse files Browse the repository at this point in the history
- still need to mirror it onto the frontend if peer review is requested
from within the vue component (tagged with a FIXME)
- correct test call site, should look into refactoring the fs module(s)
at some point + add more test coverage
  • Loading branch information
alee committed Oct 19, 2023
1 parent a3990ca commit 424cea6
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 37 deletions.
2 changes: 1 addition & 1 deletion django/curator/tests/test_dump_restore.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def setUp(self):
fs_api = self.release.get_fs_api()
import_archive(
codebase_release=self.release,
nestedcode_folder_name="library/tests/archives/nestedcode",
nested_code_folder_name="library/tests/archives/nestedcode",
fs_api=fs_api,
)

Expand Down
15 changes: 10 additions & 5 deletions django/library/fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@

logger = logging.getLogger(__name__)

"""
FIXME: consider refactoring to use pathlib.Path throughout this module instead of string paths
"""


class StagingDirectories(Enum):
# Directory containing original files uploaded (such as zip files)
Expand Down Expand Up @@ -141,10 +145,7 @@ def has_errors(self):

def serialize(self):
"""Return a list of message along with message level"""
logs = []
for msg in self.msgs:
logs.append(msg.serialize())
return logs, self.level
return [msg.serialize() for msg in self.msgs], self.level


class Message:
Expand All @@ -168,6 +169,10 @@ def create_fs_message(detail, stage: StagingDirectories, level: MessageLevels):


class CodebaseReleaseStorage(FileSystemStorage):
"""
storage abstraction for CodebaseRelease files
"""

stage = None

def __init__(
Expand All @@ -187,7 +192,7 @@ def __init__(
self.mimetype_mismatch_message_level = mimetype_mismatch_message_level

def validate_system_file(self, name, content) -> Optional[Message]:
# FIXME: do we expect validate_file being run on full paths?
# FIXME: do we expect validate_file to be run on absolute paths?
if fs.has_system_files(name):
return self.error(f"Ignored system file '{name}'")
return None
Expand Down
56 changes: 32 additions & 24 deletions django/library/jinja2/library/review/includes/reminders.jinja
Original file line number Diff line number Diff line change
@@ -1,45 +1,53 @@
<div class="alert alert-danger mb-4" role="alert">
<p>
<i class='fas fa-exclamation-triangle'></i> Peer reviews may only occur on
<i class='fas fa-exclamation-triangle'></i> Peer reviews must be requested on
<strong>unpublished model releases</strong> so that you can continue to revise your submission based on
reviewer guidance.
</p>
<p class="mb-0">
If you have already published your model <strong>a new draft release will be created</strong> as an
exact copy. On completion, the release can then be published to supersede unreviewed versions.
exact copy of your most recent published release. If your model successfully passes peer review you can then
choose to publish the reviewed release as the latest version.
</p>
</div>
<p>Before submitting your peer review request, please ensure that your model conforms to the following <a
href="/reviews/" target="_blank">review criteria</a>:</p>
<p>Before submitting your peer review request, please ensure that your model conforms to the following
<a href="/reviews/" target="_blank">review criteria</a>:
</p>
<ul class="mb-4">
<li>
<strong>Clean source code</strong> that is well-commented, uses meaningful variable names, consistently
formatted, etc.
<strong>Has "clean" source code</strong> that uses meaningful variable, method, and/or class names, is
well-structured and consistently formatted for readability, has thorough comments that describe the
semantics and intent of the code, and is in general
<q cite='https://doi.org/10.1038/d41586-018-05004-4'>as simple as possible but no simpler</q>. The
ideal goal we are striving for here is to make it as easy as possible for others (including your future self) to
understand your code.
</li>
<li>
<strong>Detailed narrative documentation</strong> containing equations, flowcharts, or other diagrams in
a structured format like the <a href="https://www.ufz.de/index.php?de=40429" target="_blank">ODD protocol</a> or
equivalent. Published journal articles generally are not considered sufficient narrative documentation
<strong>Has detailed narrative documentation</strong> containing equations, flowcharts, or other
diagrams in a structured format like the
<a href="https://www.ufz.de/index.php?de=40429" target="_blank">ODD protocol</a> or
equivalent. We <strong>do not recommend submitting a published journal article</strong> as your model's
narrative documentation as they are often not detailed enough to allow others to fully reproduce your model
and pose copyright issues for us to host.
</li>
<li>
<strong>Runnable</strong> without any changes to the downloaded package. Take care to remove absolute
paths from your source code and see the following note for referencing data files.
<strong>Is easily runnable</strong> without substantive changes to the downloaded package. Make sure to remove
any absolute paths (e.g., <code>/home/twubc/project/psyche/2023-01-14/data/hurn.nc</code> or
<code>C:\Users\twubc\Downloads\data\ferns-and-wells.zip</code> or <code>/Users/twubc/data/ferns.nc</code>)
from your source code and read the following note carefully for more details on how to reference any data
files you upload to the Computational Model Library.
</li>
</ul>
<div class="alert bg-gray" role="alert">
<p><strong>Note on Uploaded Data</strong></p>
<p class="mb-0"><small>CoMSES Net stores uploaded data files in
<code>&lt;project-root&gt;/data/</code> and code in
<code>&lt;project-root&gt;/code/</code>. This means that references to your uploaded data
files within your model will typically be via a relative path like
<code>"../data/my-input-data.csv"</code>. If you uploaded a zipfile with a nested directory
structure you may need to go up several directories to get to the project root before
descending back down into the data directory, depending on where your source code files
exist within that nested directory structure (e.g., a Python script in a
<code>src/python/</code> directory would have to reference uploaded data files in
<code>"../../../data/"</code>). This can be mitigated by including your data in your source
code zipfile and referencing relative paths to your input data directly in your code and
ignoring the CoMSES data upload slot.
</small></p>
<p class="mb-0">CoMSES Net stores uploaded data files in <code>&lt;project-root&gt;/data/</code> and code in
<code>&lt;project-root&gt;/code/</code>. This means that your model will need to reference uploaded data
files via a relative path like <code>"../data/my-input-data.csv"</code>. If you uploaded a zipfile with a
nested directory structure you may need to go up several directories to get to the project root before
descending back down into the data directory, depending on where your source code files exist within that nested
directory structure. For example, a Python script in a <code>src/python/</code> directory would have to
reference uploaded data files in <code>"../../../data/"</code>). This limitation can be sidestepped by including
your data in your source code zipfile and referencing relative paths to your input data directly in your code
and ignoring the CoMSES data upload slot.
</p>
</div>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
The CoMSES Net Computational Model Peer Review process is not intended to be time-intensive and consists of a simple checklist to verify that a computational model's source code and documentation meets baseline standards derived from ["good enough practices"]({{build_absolute_uri("/resources/guides-to-good-practice/")}}) in the software engineering and scientific communities we serve. Through this process we hope to foster higher quality models shared in the community for reuse, reproducibility, and advancement of the field in addition to supporting the [emerging practice of software citation](https://www.force11.org/software-citation-principles).
The CoMSES Net Computational Model Peer Review process is not intended to be time-intensive and consists of a simple checklist to verify that a computational model's source code and documentation meets baseline standards derived from [*good enough practices*]({{build_absolute_uri("/resources/guides-to-good-practice/")}}) in the software engineering and scientific communities we serve. Through this process we hope to foster higher quality models shared in the community for reuse, reproducibility, and advancement of the field in addition to supporting the [emerging practice of software citation](https://www.force11.org/software-citation-principles).

Reviewers should evaluate the computational model according to the following criteria:

Expand Down
11 changes: 5 additions & 6 deletions django/library/tests/test_fs.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import shutil
from pathlib import Path

import os
from django.test import TestCase
Expand All @@ -11,13 +11,13 @@
)
from library.fs import (
FileCategoryDirectories,
ArchiveExtractor,
StagingDirectories,
MessageLevels,
import_archive,
)
from library.tests.base import CodebaseFactory


import logging

logger = logging.getLogger(__name__)
Expand All @@ -28,7 +28,7 @@ def setUpModule():


class ArchiveExtractorTestCase(TestCase):
nestedcode_folder_name = "library/tests/archives/nestedcode"
nested_code_folder = Path("library/tests/archives/nestedcode")

def setUp(self):
self.user_factory = UserFactory()
Expand All @@ -41,7 +41,7 @@ def test_zipfile_saving(self):
fs_api = self.codebase_release.get_fs_api()
msgs = import_archive(
codebase_release=self.codebase_release,
nestedcode_folder_name=self.nestedcode_folder_name,
nested_code_folder_name=str(self.nested_code_folder),
fs_api=fs_api,
)
logs, level = msgs.serialize()
Expand Down Expand Up @@ -85,8 +85,7 @@ def test_invalid_zipfile_saving(self):
@classmethod
def tearDownClass(cls):
super().tearDownClass()
if os.path.exists(cls.nestedcode_folder_name):
os.remove("{}.zip".format(cls.nestedcode_folder_name))
cls.nested_code_folder.with_suffix(".zip").unlink(missing_ok=True)


def tearDownModule():
Expand Down
1 change: 1 addition & 0 deletions frontend/src/components/releaseEditor/ReviewModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
>
<template #body>
<div class="alert alert-danger mb-4" role="alert">
<!-- FIXME: is there a way to deduplicate this + reminders.jinja -->
<p>
<i class="fas fa-exclamation-triangle"></i> Peer reviews may only occur on
<strong>unpublished model releases</strong> so that you can continue to revise your
Expand Down

0 comments on commit 424cea6

Please sign in to comment.