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

Create Method to get the OIFITS standard of the OIFitsFile #12

Closed
Hitogoroshi opened this issue Jan 26, 2018 · 12 comments
Closed

Create Method to get the OIFITS standard of the OIFitsFile #12

Hitogoroshi opened this issue Jan 26, 2018 · 12 comments
Assignees
Labels
enhancement New feature or request

Comments

@Hitogoroshi
Copy link
Contributor

Hitogoroshi commented Jan 26, 2018

FitsConstants.KEYWORD_CONTENT_OIFITS2.equals(FitsConstants.KEYWORD_CONTENT) ? OIFitsStandard.VERSION_2 : OIFitsStandard.VERSION_1) is used 4 time !

Proposal:
/**
* define the version of the file
*
* @return the version
*/
public OIFitsStandard defineOIFITSVersion() {
return (FitsConstants.KEYWORD_CONTENT_OIFITS2.equals(FitsConstants.KEYWORD_CONTENT) ? OIFitsStandard.VERSION_2 : OIFitsStandard.VERSION_1);
}

@Hitogoroshi Hitogoroshi added the enhancement New feature or request label Jan 26, 2018
@Hitogoroshi Hitogoroshi self-assigned this Jan 26, 2018
@bourgesl
Copy link
Member

It looks strange !

The expression "FitsConstants.KEYWORD_CONTENT_OIFITS2.equals(FitsConstants.KEYWORD_CONTENT) ? OIFitsStandard.VERSION_2 : OIFitsStandard.VERSION_1)" is CONSTANT !

C1.equals(C2) ? V1 : V2 => V1 or V2 !

Please verify the use cases.

You would need instead:
public OIFitsStandard getOIFitsStandard(String content) {
return (FitsConstants.KEYWORD_CONTENT_OIFITS2.equals(content) ? OIFitsStandard.VERSION_2 : OIFitsStandard.VERSION_1);
}

@Hitogoroshi
Copy link
Contributor Author

Right.
Last problem is where to place the method.
We have a use like this when oifitsfile does not exist yet.

@bourgesl
Copy link
Member

Make a static public method in the appropriate class ...

"We have a use like this when oifitsfile does not exist yet."
Alright, what is your proposal ?

@Hitogoroshi
Copy link
Contributor Author

In OIFITSFILE:
use : (FitsConstants.KEYWORD_CONTENT_OIFITS2.equals(FitsConstants.KEYWORD_CONTENT) ? OIFitsStandard.VERSION_2 : OIFitsStandard.VERSION_1) to define version. (2 times)

In OIFITSLOADER:
use: (FitsConstants.KEYWORD_CONTENT_OIFITS2.equals(content) ? OIFitsStandard.VERSION_2 : OIFitsStandard.VERSION_1) to define version of OIFITS
with content = header.getTrimmedStringValue(FitsConstants.KEYWORD_CONTENT) ( 2 times)

@bourgesl
Copy link
Member

Check & verify the OIFITSFILE as it looks buggy !

@Hitogoroshi
Copy link
Contributor Author

Hitogoroshi commented Jan 30, 2018

We have in OiFitsFile a comparison with constants (used two times):

(FitsConstants.KEYWORD_CONTENT_OIFITS2.equals(FitsConstants.KEYWORD_CONTENT) ? OIFitsStandard.VERSION_2 : OIFitsStandard.VERSION_1)

We need to check its effect

Proposal:
Can delete the method : checker.inspectRuleFailed() (Special case which catch the failures using directly the applyTo and version information)

To

checker.ruleFailed(Rule.OIFITS_OI_TARGET_EXIST);
which now can process rules begin with OIFITS (And add "FILE" in the applyTo for this rules).

@Hitogoroshi
Copy link
Contributor Author

Question related to the review of the method:
inspectRuleFailed(final Rule rule, final String applyTo, final OIFitsStandard standard)

Maybe new issue

@bourgesl
Copy link
Member

I still do not understand anything...

@gmella
Copy link
Member

gmella commented Jan 31, 2018

Yes, what is the question related to the review of the method ?

@Hitogoroshi
Copy link
Contributor Author

Create Method to define version of the File in OIFitsFile done:
getOIFitsStandard(content)

@bourgesl
Copy link
Member

still waiting for patch

@bourgesl bourgesl reopened this Jan 31, 2018
@bourgesl bourgesl changed the title Create Method to define version of the File (Loading file) Create Method to get the OIFITS version of the OIFitsFile Jan 31, 2018
@bourgesl bourgesl changed the title Create Method to get the OIFITS version of the OIFitsFile Create Method to get the OIFITS standard of the OIFitsFile Jan 31, 2018
Hitogoroshi pushed a commit to Hitogoroshi/OITools that referenced this issue Jan 31, 2018
Hitogoroshi pushed a commit to Hitogoroshi/OITools that referenced this issue Jan 31, 2018
bourgesl pushed a commit that referenced this issue Jan 31, 2018
* Create Method to get the OIFITS standard of the OIFitsFile #12
@bourgesl
Copy link
Member

Fix pushed

bourgesl pushed a commit that referenced this issue Feb 1, 2018
* Create Method to define version of the File (Loading file) #12

* Create Method to get the OIFITS standard of the OIFitsFile #12

* Create Method to get the OIFITS standard of the OIFitsFile #12

* Junit Test loading lots of files when isInspectRule is active #4

* Junit Test loading lots of files when isInspectRule is active #4

* Junit Test loading lots of files when isInspectRule is active #4

* Junit Test loading lots of files when isInspectRule is active #4
bourgesl pushed a commit that referenced this issue Feb 9, 2018
* Create Method to define version of the File (Loading file) #12

* Create Method to get the OIFITS standard of the OIFitsFile #12

* Create Method to get the OIFITS standard of the OIFitsFile #12

* Junit Test loading lots of files when isInspectRule is active #4

* Junit Test loading lots of files when isInspectRule is active #4

* Junit Test loading lots of files when isInspectRule is active #4

* Junit Test loading lots of files when isInspectRule is active #4

* bash file for merge upstream and master

* JavaDoc

* Failures diagramm

* diagram UML for load information and load/write files

* some JavaDoc fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants