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

Changes to Data class attributes and TrajectoryData data storage #2422

Conversation

elsapassaro
Copy link
Contributor

@elsapassaro elsapassaro commented Jan 25, 2019

Fixes #201

Added migrations and tests for TrajectoryData symbols (from numpy array to attribute).
In sqlalchemy migrations and test I used load_node; to discuss if we want to rewrite it to avoid using aiida functionality (e.g. to get and delete numpy arrays).

@coveralls
Copy link

coveralls commented Jan 25, 2019

Coverage Status

Coverage increased (+4.6%) to 68.224% when pulling 5355a8b on asle85:fix_201_method_names_for_data into c6580e6 on aiidateam:provenance_redesign.

@sphuber sphuber force-pushed the fix_201_method_names_for_data branch from 90130f2 to 2ef384d Compare February 8, 2019 10:23
@sphuber sphuber changed the title [WIP] Fix 201 method names for data Changes to Data class attributes and TrajectoryData data storage Feb 8, 2019
@sphuber sphuber force-pushed the fix_201_method_names_for_data branch from 2ef384d to 81c5794 Compare February 8, 2019 11:45
@sphuber
Copy link
Contributor

sphuber commented Feb 8, 2019

@giovannipizzi and @asle85 I have cleaned Elsa's commits and squashed them into a single one on top of Andreas'. Since this was quite a complicated process due to various merge actions having created duplicate commits, there may have been some changes that have been lost. I doubt it but it would be good if you guys can check if in the changes everything is still there.

@sphuber sphuber force-pushed the fix_201_method_names_for_data branch from 81c5794 to 3b8556e Compare February 8, 2019 12:24
Methods changed to properties:

 * `Code.is_hidden`
 * `RemoteData.is_empty`

Name changes:

 * `ArrayData.iterarrays` to `ArrayData.get_iterarrays`
 * `Code.full_text_info` to `Code.get_full_text_info`
 * `Code.is_hidden` to `Code.hidden`
 * `StructureData._get_cif` to `StructureData.get_cif`
 * `TrajectoryData._get_aiida_structure` to
 * `TrajectoryData.get_structure`
 * `TrajectoryData._get_cif` to `TrajectoryData.get_cif`

Data structure storage changes:

 * Symbols in `TrajectoryData` are now passed as a list instead of
 * `numpy.ndarray`
 * Symbols in `TrajectoryData` are now stored as an attribute instead of
 * an array in the repository
 * `TrajectoryData.set_trajectory` arguments `stepid` and `cells` are now optional
   Change `TrajectoryData` to make passing stepids and cells optional. While
   nothing will be stored for cells if not given a consecutive sequence will be
   automatically assigned to stepids if missing from the inputs.
@sphuber sphuber force-pushed the fix_201_method_names_for_data branch 2 times, most recently from f9143b5 to b515dc6 Compare February 8, 2019 13:13
In the previous commit, the ORM for `TrajectoryData` was changed to
start storing the `symbols` as a list in the attributes of the node
instead of a `numpy` array in the repository, to make it possible to
query for the symbols. This requires a data migration which is
implemented in this commit.

Since it involves the deletion of the data from the repository after it
has been copied to the database, the migration is performed in two
stages. The first will copy the data from the repository to the
attributes on the node and the second will delete the data in the
repository. In this way, if an exception occurs during the migration, no
data will be lost.
@sphuber sphuber force-pushed the fix_201_method_names_for_data branch from b515dc6 to 5355a8b Compare February 8, 2019 13:32
@sphuber sphuber merged commit a000899 into aiidateam:provenance_redesign Feb 8, 2019
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.

4 participants