nexusformat / definitions

Definitions of the NeXus Standard File Structure and Contents
https://manual.nexusformat.org/
Other
26 stars 55 forks source link

Nyaml2nxdl migration #1303

Open sanbrock opened 1 year ago

sanbrock commented 1 year ago

This PR has been composed from PR #1272, and #1282 with also conserving micro-crediting along the development history.

As comments describe, the 2023 Code Camp participants suggested to

Since there was an intersection of the codes required for both PRs, it was easier to pull the code and its changes together into this new PR.

sanbrock commented 1 year ago

Other issues:

  • Use lxml instead of the standard xml.etree.ElementTree
  • Use tuples instead of lists your literals
  • Sort out licensing to be compatible with LGPL v3
  • Add proper pydocs
  • Re-format with black
  • Consider adding type annotation

Thank you for the nice review and pointing out all the possibilities for improvement. We can do a few quick fixes, but do not have resources for redesign, like using lxml instead of etree, etc.
We were happily copying over our implementation from pynxtools to here, but we cannot do the full set of suggestions for better implementation. Shall we accept the code more or less as is, or shall we just use the nyaml2nxdl tool in the Makefile directly from pynxtools?

PeterC-DLS commented 1 year ago

The switch to lxml is trivial.

I'm concerned about code quality and its effect on maintainability. There is little redesign needed - most of my comments are to fix minor issues with the un-Pythonic coding.

Note, this PR does not pass CI as in its present state.

RubelMozumder commented 9 months ago

@sanbrock, @PeterC-DLS, and @prjemian, the requested changes or proposals in this PR have been modified. The changes are marked by :+1: or a comment on the change request. I tried to explain why a few proposals have not been accepted.

prjemian commented 9 months ago

I'm concerned. Comments were entered, changes were made, it is not obvious how some of the comments were addressed. In some cases, entire methods were removed from the code. No mention of this in the review comments. Please show how each comment is addressed, either by code changes or other disposition.

The new support for NYAML appears substantial, both in impact and in code size. Without proper follow-up, this PR becomes very difficult to review.

RubelMozumder commented 9 months ago

I'm concerned. Comments were entered, changes were made, it is not obvious how some of the comments were addressed. In some cases, entire methods were removed from the code. No mention of this in the review comments. Please show how each comment is addressed, either by code changes or other disposition.

The new support for NYAML appears substantial, both in impact and in code size. Without proper follow-up, this PR becomes very difficult to review.

@prjemian and @PeterC-DLS sorry for being this PR ambiguous.

Some code has been removed entirely, as the code is irrelevant to this converter tool. Then the question is, why these irrelevant codes are in this PR? To explain the removal of code, I have to go a bit back to the history of the converter. Initially, was developed by another developer but was not working perfectly. Later, the entire code was modified, reshaped, and overwritten quickly because we had many yaml to convert to the nxdl.xml version of the definition. One functionality append_yml was planned to be removed which does not work. The functionality of append_yml to extend the existing nxdl.xml file if any changes come in the yaml version of the definition. Simply the nyaml2nxdl converter can be rerun on the yaml definition to generate nxdl.xml better than using the append_yml function. So, the entire code associated with that function has been removed.

Otherwise, no other code has been disposed of. Only some code parts have been shifted to nyaml2ndl_helper.py which is the reflection of PR comment. Especially, If see any comments can also be implemented on the other parts of the code and see utilizing nyaml2ndl_helper.py to keep the code more maintainable and manageable.

I tried to fix all the requested changes and did not extend the discussion so as not to make the discussion longer otherwise it will be super hard to follow the comments. Already it is a lot to go through the discussion as also mentioned by @prjemian.

PeterC-DLS commented 9 months ago

I've gone through the entire conversation in this PR and resolved any comment that has been addressed.

Remaining tasks:

  1. Please check and address for unresolved comments
  2. Ensure any spelling fixes are searched for and replaced through all these files
PeterC-DLS commented 9 months ago

I've gone through resolved some conversations but there's still 25 left: image

RubelMozumder commented 9 months ago

I've gone through resolved some conversations but there's still 25 left: image

There are some changes already made but are not updated in you system. Also, replied to some of you comments where I am not changing the code, I would emphasize resolving that comment, if you found it reasonable.

PeterC-DLS commented 9 months ago

Sorry, I don't see any recent replies in those conversations.

prjemian commented 9 months ago

Review conversations/comments should be resolved by the reviewer. The author should reply to the comment how it has been addressed.

On Thu, Sep 28, 2023, 9:22 AM Peter Chang @.***> wrote:

Sorry, I don't see any replies in those conversations.

— Reply to this email directly, view it on GitHub https://github.com/nexusformat/definitions/pull/1303#issuecomment-1739342744, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARMUMAYBYAXYMX4KCJ6DRLX4WBYTANCNFSM6AAAAAA2GDIICA . You are receiving this because you were mentioned.Message ID: @.***>

RubelMozumder commented 9 months ago

Review conversations/comments should be resolved by the reviewer. The author should reply to the comment how it has been addressed. On Thu, Sep 28, 2023, 9:22 AM Peter Chang @.> wrote: Sorry, I don't see any replies in those conversations. — Reply to this email directly, view it on GitHub <#1303 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARMUMAYBYAXYMX4KCJ6DRLX4WBYTANCNFSM6AAAAAA2GDIICA . You are receiving this because you were mentioned.Message ID: @.>

Yes, you are right. I have addressed some comments, for some unknown reason, @PeterC-DLS can not follow that conversion though they are resolved.

RubelMozumder commented 9 months ago

Sorry, I don't see any recent replies in those conversations.

@PeterC-DLS, if you go through the conversations in GitHub you might notice them. Here, I have a short-screen video at https://share.vidyard.com/watch/m1edKjV9CoYiZThVgwkdRk.

PeterC-DLS commented 9 months ago

It's very strange indeed. I see this in firefox: image

I will try another browser.

PeterC-DLS commented 9 months ago

I get the same with Chrome. I noticed that your comments are pending: github-pr-pending

So you need to submit your review to make it public to all: https://github.com/orgs/community/discussions/10369

RubelMozumder commented 9 months ago

It's very strange indeed. I see this in firefox: image

I will try anoth Contradict_by_code er browser.

Here I see in Google chrome.

PeterC-DLS commented 9 months ago

@RubelMozumder, can you submit your review to make your pending comments visible? Thanks!

RubelMozumder commented 9 months ago

@RubelMozumder, can you submit your review to make your pending comments visible? @PeterC-DLS submited.

RubelMozumder commented 9 months ago

@PeterC-DLS os lib has been removed from file processing, and xml lib has also been removed by lxml lib. All the changes are available upstream.

PeterC-DLS commented 9 months ago

Great, you need to update the dependencies for lxml. Already done!

RubelMozumder commented 9 months ago

Other issues:

  • Use lxml instead of the standard xml.etree.ElementTree
  • Use tuples instead of lists your literals
  • Sort out licensing to be compatible with LGPL v3
  • Add proper pydocs
  • Re-format with black
  • Consider adding type ==>
  • [x] Use lxml instead of the standard xml.etree.ElementTree => Done
  • [x] Use tuples instead of lists your literals => Done
  • [x] Sort out licensing to be compatible with LGPL v3 => It is not a issue. As apache project can be part of a GPLv3 project. Please check here : https://www.apache.org/licenses/GPL-compatibility.html
  • [x] Add proper pydocs =>Tried as much as possible.
  • [x] Re-format with black =>Done
  • [x] Consider adding type => Though partially some of the type has been done before, but not on the entire code.
RubelMozumder commented 9 months ago

I checked for xml lib in requirement.txt and pyproject.toml files to remove from dependencies. But I do not see it in the dependencies list.

domna commented 9 months ago

Just Pete's comment about the build system needs addressing

From @prjemian s comment I read that we should move the pyproject.toml into a separate PR anyways to keep the changes visible. The packaging is, however, not working properly yet when simply installed with a pip install (w/o the -e for dev install - see https://github.com/nexusformat/definitions/pull/1303#discussion_r1335912717)

I could try fix this with the current project structure and also add the necessary changes in the github Actions (should only be minor changes). I would then create a new PR with this. @PeterC-DLS @prjemian @RubelMozumder Sounds good?

RubelMozumder commented 9 months ago

Just Pete's comment about the build system needs addressing

From @prjemian s comment I read that we should move the pyproject.toml into a separate PR anyways to keep the changes visible. The packaging is, however, not working properly yet when simply installed with a pip install (w/o the -e for dev install - see #1303 (comment))

I could try fix this with the current project structure and also add the necessary changes in the github Actions (should only be minor changes). I would then create a new PR with this. @PeterC-DLS @prjemian @RubelMozumder Sounds good?

I agree to set a different PR, in any case, these changes would not stop this #1303. And also the git history will stay clear.

domna commented 9 months ago

Just Pete's comment about the build system needs addressing

From @prjemian s comment I read that we should move the pyproject.toml into a separate PR anyways to keep the changes visible. The packaging is, however, not working properly yet when simply installed with a pip install (w/o the -e for dev install - see #1303 (comment))

Sorry, I just realized that the proposal was that we just should create a reference to an issue solving this. I think this should be no problem.

I just had a discussion with @sanbrock and @RubelMozumder on how to deal with the packaging issue of the package and we think that we probably have to move everything to a subdirectory to properly allow python to copy everything into site-packages. This would mean that we would have a structure like this:

...
pyproject.toml
nexus_definitions/contributed_definitions
nexus_definitions/base_classes
nexus_definitions/applications
nexus_definitions/dev_tools
nexus_definitions/nxdl.xsd
nexus_definitions/nxdlTypes.xsd
nexus_definitions/NXDL_VERSION
....

There might be other files we want to copy into the nexus_definitions module. This structure in the python world is equivalent to saying that everything under nexus_definitions is part of the module and everything outside is ignored or metadata of the package.

Unfortunately, this seems to be the only solution to the problem as python always assumes that the modules are kept in subfolders. I already searched whether there is a pyproject.toml setting which lets setuptools copy the whole project to site-packages but I did not find anything. I can try to do another search but I think this is just the way python packaging is intended to work.

So if you @prjemian and @PeterC-DLS agree I can prepare the following in this PR:

What do you think? If you're happy I would proceed with the points (+ feel free to add anything related I forgot).

PeterC-DLS commented 9 months ago

Thanks for the update, Florian. I think it's best to make this repacking another PR. This is an issue on what are the requirements for making a Python package out of any portion of this repo. What are the use-cases and workflows intended for this new package?

prjemian commented 9 months ago

This needs a broader discussion now. The NeXus definitions repository is for the definition of the NeXus data standard and its documentation. Until now, the definitions repository provides no content suitable for for distribution as a Python package.

The proposition, as I understand it now, is to introduce tools to convert files of the nyaml format into NXDL as NeXus classes. (It is noted this comes with some possibility that our community will see nyaml as an alternative method to define NeXus classes. The NIAC should review that new possibility.)

Now, a new conversation suggests that, for the purposes of nyaml files, we refactor the entire definitions repository into a Python package for distribution.

Perhaps an alternative is possible to avoid major changes in the definitions repo for nyaml files. Make the nyaml conversion tools a separate repository. That repository can be developed as a Python package and used by the definitions repo as needed. With such a separate package, this entire PR can be moved to the new repo and developed outside of the definition of the NeXus standard.

RubelMozumder commented 9 months ago

Just Pete's comment about the build system needs addressing

Sytem is building without any issue, now.

sanbrock commented 9 months ago

This needs a broader discussion now. The NeXus definitions repository is for the definition of the NeXus data standard and its documentation. Until now, the definitions repository provides no content suitable for for distribution as a Python package.

The proposition, as I understand it now, is to introduce tools to convert files of the nyaml format into NXDL as NeXus classes. (It is noted this comes with some possibility that our community will see nyaml as an alternative method to define NeXus classes. The NIAC should review that new possibility.)

Now, a new conversation suggests that, for the purposes of nyaml files, we refactor the entire definitions repository into a Python package for distribution.

Perhaps an alternative is possible to avoid major changes in the definitions repo for nyaml files. Make the nyaml conversion tools a separate repository. That repository can be developed as a Python package and used by the definitions repo as needed. With such a separate package, this entire PR can be moved to the new repo and developed outside of the definition of the NeXus standard.

nyaml:

python module:

merged PR:

prjemian commented 9 months ago

AFAIK, a code camp is not a NIAC meeting.

On the codecamp ...

No reference to that here until this comment. Such references would help to understand the context of these changes and to help review.

The first section of the User Guide's Introduction describes the purpose of the NeXus standard. We have worked diligently to keep implementation of the standard separate from any one computer language. This PR has become a clear example of scope creep.

domna commented 9 months ago

I completely removed the packaging parts and adapted the Makefile that it works without being a package. So it should be good to go from the packaging side now.

prjemian commented 8 months ago

First, update this PR's description to describe the work being done here. The work in this branch goes far beyond what is described.

micro-crediting is also conserved

prjemian commented 8 months ago

Also, authors of this branch should not be in the list of reviewers.

sanbrock commented 8 months ago

First, update this PR's description to describe the work being done here. The work in this branch goes far beyond what is described.

micro-crediting is also conserved

addressed

sanbrock commented 8 months ago

Also, authors of this branch should not be in the list of reviewers.

Rubel, Florian, and me as proposers will not provide review, but only address issues. Peter has requested changes and provided review. For your concerns, @prjemian , we have revoked the content which would have provided nexus definitions as a python package, and are looking forward to your further suggestions and/or approval.
Note that this PRs does not propose any change in NeXus, its definitions, or its use, but rather tries to enhance the documentation and enables a more human-friendly editing of the definitions.

prjemian commented 8 months ago

... enables a more human-friendly editing of the definitions

This is not in the original statement of this PR except by mention of #1282. Can that be separated from this PR? I would like to review documentation changes separate from the proposition of new editing tools.

prjemian commented 8 months ago

Still, I do not see any justification to make NeXus definitions become an installable Python package. This is beyond the scope of NeXus definitions and belongs in its own repository. Such a separate Python package could be used here with the other dev_tools.

I cannot approve this PR as-is. The intent is wrong.

PeterC-DLS commented 8 months ago

Hi @prjemian, there is no longer any configuration to make anything a built Python package in this PR.

RubelMozumder commented 8 months ago

I read a conversation somewhere asking about copy-right year (Copyright (C) 2014-2022 NeXus) that comes in the nexus definition. But, now I can not find it anymore.

It was hard coded by 2014-2022 as suggested by @sanbrock in the original version of nyaml2nxdl. Later, in this PR @PeterC-DLS suggested changing it to the current year rather than hard coded.

prjemian commented 8 months ago

Do not change the starting date for NeXus Copyright lines.

There is history in those dates, much of which precedes 2014. The earliest year shown is the year the NXDL was established. Annually, the ending copyright year has been updated by a script, run in response to an issue. The changes are reviewed as a PR, such as this one from 2022.

(base) prjemian@arf:~/.../NeXus/definitions$ git grep -i copyright | grep -i "(c)" | grep NeXus
Makefile:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXarchive.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXarpes.nxdl.xml:# Copyright (C) 2012-2022 NeXus International Advisory Committee (NIAC)
applications/NXcanSAS.nxdl.xml:# Copyright (C) 2012-2022 NeXus International Advisory Committee (NIAC)
applications/NXdirecttof.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXfluo.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXindirecttof.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXiqproc.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXlauetof.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXmonopd.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXmx.nxdl.xml:  # Copyright (C) 2013-2022 NeXus International Advisory Committee (NIAC)
applications/NXrefscan.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXreftof.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXsas.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXsastof.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXscan.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXspe.nxdl.xml:    # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXsqom.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXstxm.nxdl.xml:  # Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
applications/NXtas.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtofnpd.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtofraw.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtofsingle.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtomo.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtomophase.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtomoproc.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxas.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxasproc.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxbase.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxeuler.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxkappa.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxlaue.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxlaueplate.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxnb.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxrot.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/nxdlformat.xsl:     # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXaperture.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXattenuator.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXbeam.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXbeam_stop.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXbending_magnet.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcapillary.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcite.nxdl.xml:  # Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcollection.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcollimator.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcrystal.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcylindrical_geometry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXdata.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXdetector.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXdetector_group.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXdetector_module.nxdl.xml:# Copyright (C) 2013-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXdisk_chopper.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXentry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXenvironment.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXevent_data.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXfermi_chopper.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXfilter.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXflipper.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXfresnel_zone_plate.nxdl.xml:# Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXgeometry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXgrating.nxdl.xml:# Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXguide.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXinsertion_device.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXinstrument.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXlog.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXmirror.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXmoderator.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXmonitor.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXmonochromator.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXnote.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXobject.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXoff_geometry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXorientation.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXparameters.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXpdb.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXpinhole.nxdl.xml:# Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXpolarizer.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXpositioner.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXprocess.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXroot.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXsample.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXsample_component.nxdl.xml:# Copyright (C) 2016-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXsensor.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXshape.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXslit.nxdl.xml:# Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXsource.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXsubentry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXtransformations.nxdl.xml:# Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXtranslation.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXuser.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXvelocity_selector.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXxraylens.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/nxdlformat.xsl:     # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXcontainer.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXcsg.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXcxi_ptycho.nxdl.xml:  # Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXelectrostatic_kicker.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXmagnetic_kicker.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXquadric.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXquadrupole_magnet.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXregion.nxdl.xml:# Copyright (C) 2013-2021 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXseparator.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXsolenoid_magnet.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXsolid_geometry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXspin_rotator.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXxpcs.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/nxdlformat.xsl:     # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
manual/source/copyright.rst:Copyright (C) 1996-2022 NeXus International Advisory Committee (NIAC)
manual/source/examples/NX__template__.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
manual/source/examples/NXwoni.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
nxdl.xsd:               Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
nxdlTypes.xsd:              Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
utils/create_release_notes.py:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
utils/update_copyright_date.py:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
prjemian commented 8 months ago

Please finish the work for the stated goals on this PR or withdraw it. It cannot be reviewed until the authors have finished with their intended changes. After review, no changes should be made that do not address the review comments. All reviewer comments should be addressed. It is the reviewer's responsibility to close a review conversation once the reviewer deems the conversation is complete. PR authors should not close these conversations.

Still, this reviewer question remains unanswered:

... enables a more human-friendly editing of the definitions

This is not in the original statement of this PR except by mention of #1282. Can that be separated from this PR? I would like to review documentation changes separate from the proposition of new editing tools.

sanbrock commented 8 months ago

Please finish the work for the stated goals on this PR or withdraw it. It cannot be reviewed until the authors have finished with their intended changes. After review, no changes should be made that do not address the review comments. All reviewer comments should be addressed. It is the reviewer's responsibility to close a review conversation once the reviewer deems the conversation is complete. PR authors should not close these conversations.

Still, this reviewer question remains unanswered:

... enables a more human-friendly editing of the definitions

This is not in the original statement of this PR except by mention of #1282. Can that be separated from this PR? I would like to review documentation changes separate from the proposition of new editing tools.

The initial aim of this PR is summarised in its description: merge the suggestions from 1272 and 1282 into one PR, because both were dependent on a code from pynxtools which was requested to be moved into definitions repo in nexusformat rather then to use it as dependency. Hence, keeping the 2 PRs separate would have ment to make requested modifications of separate reviews on the same code parallel in two different PRs, in two different branches. To simplify the management of the reviews, it was suggested to close the original PRs and rather request a new one:

This PR has been composed from PR https://github.com/nexusformat/definitions/pull/1272, and https://github.com/nexusformat/definitions/pull/1282 with also conserving micro-crediting along the development history.

https://github.com/nexusformat/definitions/pull/1272 aimed enhancements for the documentation by providing an extra link if applicable to the first vocabulary item referenced (e.g. /NXxkappa/entry/sample-group should reference /NXxbase/entry/sample-group and not only provide a link to /NXsample)

https://github.com/nexusformat/definitions/pull/1282 aimed to add a convenient function to the Makefile for converting definitions between nxdl.xml and nyaml and allow simpler editing of the definitions by humans in yaml format, while keeping the nxdl.xml files as the single source of truth.

RubelMozumder commented 8 months ago

Do not change the starting date for NeXus Copyright lines.

There is history in those dates, much of which precedes 2014. The earliest year shown is the year the NXDL was established. Annually, the ending copyright year has been updated by a script, run in response to an issue. The changes are reviewed as a PR, such as this one from 2022.

(base) prjemian@arf:~/.../NeXus/definitions$ git grep -i copyright | grep -i "(c)" | grep NeXus
Makefile:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXarchive.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXarpes.nxdl.xml:# Copyright (C) 2012-2022 NeXus International Advisory Committee (NIAC)
applications/NXcanSAS.nxdl.xml:# Copyright (C) 2012-2022 NeXus International Advisory Committee (NIAC)
applications/NXdirecttof.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXfluo.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXindirecttof.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXiqproc.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXlauetof.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXmonopd.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXmx.nxdl.xml:  # Copyright (C) 2013-2022 NeXus International Advisory Committee (NIAC)
applications/NXrefscan.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXreftof.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXsas.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXsastof.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXscan.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXspe.nxdl.xml:    # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXsqom.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXstxm.nxdl.xml:  # Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
applications/NXtas.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtofnpd.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtofraw.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtofsingle.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtomo.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtomophase.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXtomoproc.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxas.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxasproc.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxbase.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxeuler.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxkappa.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxlaue.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxlaueplate.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxnb.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/NXxrot.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
applications/nxdlformat.xsl:     # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXaperture.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXattenuator.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXbeam.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXbeam_stop.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXbending_magnet.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcapillary.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcite.nxdl.xml:  # Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcollection.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcollimator.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcrystal.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXcylindrical_geometry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXdata.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXdetector.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXdetector_group.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXdetector_module.nxdl.xml:# Copyright (C) 2013-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXdisk_chopper.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXentry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXenvironment.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXevent_data.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXfermi_chopper.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXfilter.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXflipper.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXfresnel_zone_plate.nxdl.xml:# Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXgeometry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXgrating.nxdl.xml:# Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXguide.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXinsertion_device.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXinstrument.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXlog.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXmirror.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXmoderator.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXmonitor.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXmonochromator.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXnote.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXobject.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXoff_geometry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXorientation.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXparameters.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXpdb.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXpinhole.nxdl.xml:# Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXpolarizer.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXpositioner.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXprocess.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXroot.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXsample.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXsample_component.nxdl.xml:# Copyright (C) 2016-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXsensor.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXshape.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXslit.nxdl.xml:# Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXsource.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXsubentry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXtransformations.nxdl.xml:# Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXtranslation.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXuser.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXvelocity_selector.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/NXxraylens.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
base_classes/nxdlformat.xsl:     # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXcontainer.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXcsg.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXcxi_ptycho.nxdl.xml:  # Copyright (C) 2014-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXelectrostatic_kicker.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXmagnetic_kicker.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXquadric.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXquadrupole_magnet.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXregion.nxdl.xml:# Copyright (C) 2013-2021 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXseparator.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXsolenoid_magnet.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXsolid_geometry.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXspin_rotator.nxdl.xml:  # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/NXxpcs.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
contributed_definitions/nxdlformat.xsl:     # Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
manual/source/copyright.rst:Copyright (C) 1996-2022 NeXus International Advisory Committee (NIAC)
manual/source/examples/NX__template__.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
manual/source/examples/NXwoni.nxdl.xml:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
nxdl.xsd:             Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
nxdlTypes.xsd:                Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
utils/create_release_notes.py:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)
utils/update_copyright_date.py:# Copyright (C) 2008-2022 NeXus International Advisory Committee (NIAC)

@prjemian,The files mentioned here are not part of this PR and no change has come from this PR. The copyright years you mentioned in those files are originally the main branch.

prjemian commented 8 months ago

Right. Those changes are PR #1322

RubelMozumder commented 8 months ago

Right. Those changes are PR #1322

PR #1322 is closed. That was created by mistake and was intended to fairmat branch

phyy-nx commented 7 months ago

Feedback from Telco:

This will greatly simplify this PR and it lets us keep the code more organized. Thanks for the work!

sanbrock commented 7 months ago

as suggested the PR are split to two again: